fix(data-table): filter placement/icon default, pinned-column gap, column-settings search & scroll - #420
Open
itsprade wants to merge 6 commits into
Open
fix(data-table): filter placement/icon default, pinned-column gap, column-settings search & scroll#420itsprade wants to merge 6 commits into
itsprade wants to merge 6 commits into
Conversation
…column-settings search & scroll
- Filters: render the Add-filter trigger on the left by default (chips flow
right; popover anchors start) and make it icon-only by default
(addIconOnly now defaults to true; pass addIconOnly={false} for the label).
- Pinned columns: measure header widths with getBoundingClientRect().width so
accumulated sticky offsets land flush and no sub-pixel gap leaks scrolling
rows between adjacent frozen columns.
- ColumnSettings: add a search box that filters the Scrollable list only
(pinned zones stay in full); cap only the Scrollable list's height with a
thin, overlay, auto-hiding scrollbar that reserves no width (checkboxes stay
aligned); keep drag-to-reorder / drag-between-zones working while searching
via full-order drop indices; pin the Show/Hide-all footer.
- Lab example seeded with 22 columns to exercise the popup; i18n keys added.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
itsprade
marked this pull request as draft
August 5, 2026 09:07
IzumiSy
marked this pull request as ready for review
August 6, 2026 05:23
commit: |
…-column hug - AddFilterPanel: add an always-on search box over the field picker so a field is quick to find in tables with many filterable fields; the field column hugs the field-name width up to a 360px cap (long names truncate) while the value editor keeps a fixed 260px width — so a wide field column grows the panel rightward instead of squeezing the editor. - TruncatedLabel: clipped field/column names in both the add-filter and column-settings pickers reveal their full text in a tooltip on hover (2s). - ColumnSettings: search now narrows only the Scrollable list (pinned zones stay in full); popup width capped so long names truncate. - Extract shared useAutoHideScroll hook + autoHideScrollbarClasses (thin overlay auto-hiding scrollbar) used by both the column-settings list and the add-filter field list; add TruncatedLabel component. - i18n: add searchFields / noFieldsMatch (en + ja). - Lab example: seed 22 columns (all filterable, some long labels) to exercise the pickers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
/review |
Contributor
|
✅ API Design Review completed successfully! Review complete. Posted 2 inline comments on PR #420 (toolbar.tsx:134 and toolbar.tsx:272) flagging: (1) silent behavioral breaking changes (addIconOnly default flip + trigger reorder) that warrant a minor version bump rather than patch, and (2) a UX bug where the field list and editor panel desynced when the field search filters out the active field. Verdict: Request Changes. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Generated by API Design Review for issue #420 · 76.9 AIC · ⌖ 6.17 AIC · ⊞ 5.8K
Comment /review to run again
IzumiSy
requested changes
Aug 7, 2026
…on with search Address review feedback on #420: - Remove the custom auto-hide overlay scrollbar (useAutoHideScroll + autoHideScrollbarClasses) from the column-settings and add-filter lists; use native browser scrolling instead (per reviewer — avoid custom scrollbar complexity). Deletes use-autohide-scroll.ts. - Fix add-filter field/editor desync: when the field search filters out the currently-selected field, advance the selection to the first still-visible field so the list highlight and the value editor stay in sync. Adds a test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…b.com:tailor-platform/app-shell into fix/data-table-filter-placement-and-column-ui
IzumiSy
requested changes
Aug 7, 2026
Address review feedback: drop the TruncatedLabel component (useEffect + ResizeObserver to conditionally show a tooltip) in favor of the native `title` attribute on the field/column labels in the add-filter and column-settings pickers. Simpler, no JS, browser shows the full text on hover. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Polish and bug fixes for the DataTable filter and column show/hide UI. Patch release for
@tailor-platform/app-shell.What's improved
Filter (
DataTable.Filters)<DataTable.Filters />gets this automatically.addIconOnlynow defaults totrue(label preserved asaria-label). PassaddIconOnly={false}to show the "Add filter" text label.Pinned / sticky columns
getBoundingClientRect().width(fractional) instead of the integer-roundedoffsetWidth, so accumulated sticky offsets land flush.Column settings popup (
ColumnSettings)Notes / decisions
ColumnSettingsa matching icon-only mode if we want them symmetric.searchColumns/noColumnsMatchi18n keys added (en + ja).Testing
pnpm test— 1435 pass (added coverage: search filtering, pinned-zones-not-filtered, drag-while-searching into a pinned zone, icon-only default +addIconOnly={false}label).pnpm type-check,pnpm lint(0 errors),pnpm fmt— all clean.🤖 Generated with Claude Code