Skip to content

fix(results): fit result grid columns to header content (#1101) - #1113

Merged
cevheri merged 4 commits into
libredb:mainfrom
igorwfaoro:feat/1101-fit-result-column-headers
Sep 25, 2026
Merged

fix(results): fit result grid columns to header content (#1101)#1113
cevheri merged 4 commits into
libredb:mainfrom
igorwfaoro:feat/1101-fit-result-column-headers

Conversation

@igorwfaoro

@igorwfaoro igorwfaoro commented Sep 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

The Results Grid used one fixed initial width for every result column, so headers could be compressed or appear uniformly sized. This change sizes each desktop result column from the visible header content while leaving cell rendering and user resizing behavior unchanged.

Type of Change

  • Bug fix (non-breaking change)
  • Test addition or update
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Performance improvement

Related Issue

Closes #1101

Changes Made

  • Added a pure header-sizing utility with minimum and maximum width bounds.
  • Accounted for field names, declared types, sort/filter controls, and masking markers in the initial/reset width.
  • Preserved cell-content rendering and manual drag resizing.
  • Added double-click reset behavior for an individual resize handle.
  • Added a field-name tooltip for headers that exceed the width cap.
  • Documented the header-fit and double-click reset behavior in docs/FEATURES.md.
  • Added unit and component coverage for header sizing, value independence, typed/masked headers, long names, manual resize, and reset behavior.

Testing

  • I have tested this locally
  • I have added/updated tests
  • All existing tests pass

Test Environment

  • LibreDB Studio version: 0.16.2
  • Browser: Chromium through playwright-cli
  • OS: Linux
  • Bun: 1.3.14
  • Database: built-in Sample (Employees), SQLite

Checks run

  • bun tests/run-tests.ts tests/unit/components/results-grid-column-sizing.test.ts tests/components/ResultsGrid.test.tsx — 106/106 passed
  • bunx tsc --noEmit --pretty false — passed
  • bunx biome check on changed code files — passed
  • bun run lint — 0 errors; existing warnings remain
  • bun run build — passed
  • Browser UAT: ran SELECT * FROM employee against Sample (Employees); all six headers rendered without truncation and with distinct widths.

The full bun run test command was also attempted. It currently reports unrelated/environment-sensitive failures in 16 files under the local Bun 1.3.14 environment; neither changed ResultsGrid test file failed.

Screenshots (if applicable)

Browser validation was performed with the built-in Employees sample. No screenshot is attached because the behavior is covered by the automated DOM assertions and the live UAT.

Checklist

  • My code follows the project's code style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective
  • New and changed tests pass locally
  • The required CI test job passes the 100% line-coverage gate
  • If I changed src/lib/db/providers/, I updated the matching provider documentation and tests (n/a)
  • Any dependent changes have been merged and published (n/a)

Additional Notes

The live browser check used SELECT * FROM employee as requested. The implementation does not size columns from cell values. #1101 is the source issue and is closed by this PR.

@igorwfaoro
igorwfaoro force-pushed the feat/1101-fit-result-column-headers branch from aea0e92 to 5ecf813 Compare September 24, 2026 19:17
@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cevheri
cevheri self-requested a review September 24, 2026 19:43
@cevheri cevheri added the enhancement New feature or request label Sep 24, 2026
cevheri
cevheri requested changes Sep 24, 2026
View reviewed changes

@cevheri cevheri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Igor, this works well. I ran a 13 column query in the browser from 768 to 2560px wide: main cut off 11 headers, this branch 2. Phone layouts are unchanged, and the double-click reset works with a real mouse.

Four small things, then this is ready:

  • The double-click test still passes with table.resetColumnSizing() in place of header.column.resetSize(). Please drag both columns, double-click name, and assert that email keeps its dragged width.
  • No component case sets columnTypes or masking, so dropping those two arguments from the size call goes unnoticed. One case with a declared type and a masked column would cover it.
  • Past the 500px cap a name can never be read in full, because dragging stops at 500 too, and SQL Server allows 128 character names. Please add title={field} to the header's name span, as the type label next to it already has, and assert it.
  • Please extend the Column Management bullet in docs/FEATURES.md: a column opens at a width that fits its header, and a double-click on the resize handle restores it. One sentence per line.

@igorwfaoro

igorwfaoro commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in commit 008df5db: strengthened the per-column reset test, added typed + masked header coverage, added field-name tooltips for capped widths, and documented the behavior in docs/FEATURES.md. Focused tests now pass 106/106.

@cevheri

cevheri commented Sep 24, 2026

Copy link
Copy Markdown
Member

Thanks, all four are in, and each new test fails when I break the code it covers. One small thing left: rewriting "renders headers unchanged when the result declares no types at all" dropped its guard. With title={declaredType ?? "untyped"} on the mobile header, the old version fails and the new one passes. Please keep the old assertion and only exclude the field-name titles, for example by filtering out mockResult.fields. Then this is ready from my side.

@igorwfaoro

igorwfaoro commented Sep 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Ah, I hadn't caught that. I tested this, but missed that the previous no-types tooltip guard had been weakened. Thanks for pointing it out. I restored the guard while excluding field-name titles, and verified it fails with title={declaredType ?? "untyped"}. The focused tests pass now (106/106). Updated in commit a99a3165.

@cevheri
docs(backlog): file U48 and U49, found reviewing the result header wi…
7d1af42 
…dths in libredb#1113

The header-fit width is right for Latin names at the default font size.
Two cases it does not cover were measured in the browser during the review
and are not regressions of this change, so they are filed rather than asked
of the contributor: wide characters (U48) and a larger root font size (U49).
@cevheri

cevheri commented Sep 25, 2026

Copy link
Copy Markdown
Member

yhanks Igor, great work, and quick turnarounds on every round, I will merge this shortly

@cevheri
cevheri merged commit 8dfdfcc into libredb:main Sep 25, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cevheri cevheri cevheri approved these changes

Assignees

@igorwfaoro igorwfaoro

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEATURE] Auto-size result columns to fit their header names

2 participants

@igorwfaoro @cevheri