Skip to content

Improve data table selection#563

Merged
jordandrako merged 2 commits into
devfrom
improve-data-table-selection
Jun 1, 2026
Merged

Improve data table selection#563
jordandrako merged 2 commits into
devfrom
improve-data-table-selection

Conversation

@jordandrako
Copy link
Copy Markdown
Member

Problem

The reusable data table (frontend/src/components/ui/data-table.tsx) had three selection shortcomings:

  1. No shift+click range selection — rows could only be toggled one at a time.
  2. Selection/filter desync — selecting rows, filtering them out of view, selecting more, then clearing the filter left the checked checkboxes out of sync with the selection reported to the parent (used for bulk actions).
  3. Virtualized rows didn't reflect column/selection visibility changes — toggling selection mode or column visibility didn't update already-rendered virtual rows; you had to scroll new rows into view to see the change.

Changes

  • Shift+click range selection. In selection mode, click one checkbox then shift+click another to select the inclusive range in displayed (sorted/filtered) order. Captures shiftKey via the checkbox onClick (Radix onCheckedChange doesn't expose the event) and tracks an anchor. Select-all resets the anchor; anchors clear on exit; grouped rows are skipped; range is scoped to the current page under pagination.
  • Filter/selection desync fix. The callback now reports getSelectedRowModel() (all selected) instead of getFilteredSelectedRowModel() (only filter-visible), and columnFilters was added to the effect deps. Selections now persist across filtering and always match the checked boxes.
  • Filter-aware count. New rowsSelectedFiltered message (e.g. "5 selected (2 match filter)") avoids nonsensical "5 of 2 selected" when a filter hides selected rows. Added to en/fr/es common.json.
  • Virtualized re-render fix. visibleColumnKey was memoized on [table.getVisibleLeafColumns] — a stable function reference that never changes — so the key was computed once and never updated. Now computed inline each render; the memo comparison is by string value, so cell content still won't re-render on scroll.

i18n

Added rowsSelectedFiltered to frontend/public/locales/{en,fr,es}/common.json.

Testing

  • New frontend/src/components/ui/data-table.test.tsx — 4 tests: shift+click inclusive range, anchor reset after select-all, filter persistence sync, and the filtered count message.
  • pnpm typecheck — clean
  • biome lint — clean
  • pnpm test:run data-table tests (4/4) and locale-keys (200/200) pass

Reviewer notes

  • Documented a pre-existing manualPagination limitation in a code comment: selections on other (server) pages live in rowSelection by id but can't be reported as row objects; true cross-page selection is out of scope.
  • No consumer changes required — ProjectTasksTableView, UserPermissionsCard, and DocumentsListView all map selections to ids only.

- Add shift+click range selection in selection mode
- Fix selection/filter desync: report all selected rows, not just
  filter-visible ones, so checked boxes match the reported selection
- Add filter-aware selection count message
- Fix virtualized rows not reflecting column/selection visibility
  changes until scrolled (stale visibleColumnKey memo dependency)
- Add data-table selection tests
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 1, 2026

Greptile Summary

This PR addresses three pre-existing bugs in the reusable DataTable component and adds shift+click range selection. All four changes are well-scoped and backed by targeted tests.

  • Shift+click range selection: captures shiftKey via onClick (since Radix's onCheckedChange doesn't expose the event), stores an anchor in a ref, and applies an inclusive range over table.getRowModel().rows on the next shift+click; select-all resets the anchor; grouped rows are skipped.
  • Filter/selection desync fix: onRowSelectionChange now calls getSelectedRowModel() (all selected rows) instead of getFilteredSelectedRowModel() (filter-visible only), with columnFilters added as an effect dep so the callback fires on filter changes too.
  • Filter-aware count: added rowsSelectedFiltered i18n key (en/fr/es) and a filteredSelected < selected guard so the "X selected (Y match filter)" variant shows whenever a filter hides any selected row — including the previously-missed case where selected ≤ filteredTotal.
  • Virtualized re-render fix: replaced a useMemo(() => …, [table.getVisibleLeafColumns]) — where the dep is a stable function reference that never changes — with an inline computation so visibleColumnKey always reflects current column visibility.

Confidence Score: 5/5

All three bug fixes are narrow, correct, and covered by new tests; no regressions introduced in consumer components.

The shift+click anchor logic is correctly scoped to visible rows and properly resets on select-all. The switch to getSelectedRowModel() fixes the real desync without breaking any consumer. The inline visibleColumnKey computation is a straightforward correctness fix. The filter-count guard covers both edge cases and is verified by tests.

No files require special attention.

Important Files Changed

Filename Overview
frontend/src/components/ui/data-table.tsx Core component — three bugs fixed (shift+click range selection, filter/selection desync via getSelectedRowModel, visibleColumnKey memo dependency) plus filter-aware selection count; logic is correct and well-commented.
frontend/src/components/ui/data-table.test.tsx New test file with 5 targeted tests covering shift+click range, anchor reset after select-all, filter persistence, filtered count message, and the selected ≤ filteredTotal edge case.
frontend/public/locales/en/common.json Adds rowsSelectedFiltered key with correct {{selected}} and {{total}} placeholders; consistent with existing rowsSelected pattern.
frontend/public/locales/es/common.json Spanish translation for rowsSelectedFiltered added in the correct position.
frontend/public/locales/fr/common.json French translation for rowsSelectedFiltered added in the correct position.
CHANGELOG.md Changelog entries for the shift+click feature and filter desync fix are accurate and placed under Unreleased.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Checkbox onClick] -->|captures e.shiftKey| B[shiftKeyRef.current = shiftKey]
    B --> C[onCheckedChange fires]
    C --> D[rowCheckboxHandlerRef.current invoked]
    D --> E{isShift && value && anchorId?}
    E -->|Yes| F[Find anchor + target in visibleRows]
    F --> G{Both found?}
    G -->|Yes| H[Range-select lo..hi, skip grouped rows]
    H --> I[Update lastSelectedRowIdRef to rowId]
    G -->|No, anchor on another page| J[Single toggle via table.getRow]
    E -->|No| J
    J --> I
    I --> K[useEffect fires on rowSelection change]
    K --> L[table.getSelectedRowModel — all selected rows]
    L --> M[onRowSelectionChange reported to parent]

    subgraph Count display
        N{columnFilters active?} -->|Yes| O{filteredSelected < selected?}
        O -->|Yes| P[rowsSelectedFiltered: X selected, Y match filter]
        O -->|No| Q[rowsSelected: X of filteredTotal selected]
        N -->|No| Q
    end
Loading

Reviews (2): Last reviewed commit: "Fix misleading selection count when filt..." | Re-trigger Greptile

Comment thread frontend/src/components/ui/data-table.tsx
Switch to the filter-aware count whenever the filter hides any selected
row (filteredSelected < selected), not only when selected > filteredTotal.
Previously 'X of Y selected' could show e.g. '2 of 3 selected' while none
of the 3 visible rows were checked.
@jordandrako
Copy link
Copy Markdown
Member Author

@greptile

@jordandrako jordandrako merged commit 11ac9ae into dev Jun 1, 2026
4 checks passed
@jordandrako jordandrako deleted the improve-data-table-selection branch June 1, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant