Skip to content

feat(ui):Integrate shadcn dependency and enhance shared DataTable features - #48

Merged
unclesp1d3r merged 20 commits into
mainfrom
36-roll-up-identical-magazine-types-group-by-name-or-label-prefix-on-the-magazines-page
Jul 6, 2026
Merged

unclesp1d3r merged 20 commits into
mainfrom
36-roll-up-identical-magazine-types-group-by-name-or-label-prefix-on-the-magazines-page

Conversation

@unclesp1d3r

Copy link
Copy Markdown
Owner

This pull request introduces significant improvements to how tables are rendered and managed across several pages, replacing manual table markup with a more robust, reusable DataTable component and supporting hooks. It also removes the now-unnecessary FilterBar from the magazines page and moves filtering UI into the view itself. Additionally, a new configuration file for MCP servers is added.

Table rendering and state management modernization:

  • Replaced manual table markup in AdminUsers, RangeSessionHistory, and the summary page with the new DataTable component, using column definitions and view state hooks for consistency, sorting, and future extensibility. (app/(admin)/users/admin-users.tsx, app/(app)/firearms/range-session-history.tsx, app/(app)/summary/page.tsx) [1] [2] [3] [4] [5] [6] [7] [8]

  • Added and utilized the useTableViewState hook and related types to manage persistent table view state and column configuration. (app/(admin)/users/admin-users.tsx, app/(app)/firearms/range-session-history.tsx) [1] [2]

Magazines page filtering and UI simplification:

  • Removed the separate FilterBar component and its associated code from the magazines page, moving filtering logic and UI into the MagazinesView component for better encapsulation and a cleaner page structure. (app/(app)/magazines/filter-bar.tsx, app/(app)/magazines/page.tsx) [1] [2] [3] [4] [5]

Configuration and setup:

  • Added a new .mcp.json file to define MCP server commands, likely for local development or tooling support. (.mcp.json)

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…or magazines and firearms

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ss and refine grouping logic for magazines and firearms

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U3+U4: owner/borrowed split, count-desc/name-asc group ordering, and
member sorting in a generic buildGroups engine; magazine byType identity
and longest-prefix key selectors (AE1-AE5), capacity aggregate, and the
magazineLabelAscending default comparator; firearm byType (count-only).
Pure, bun-tested (29 tests). Carries R6-R14.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U1: headless @tanstack/react-table + hand-styled Tailwind (KTD-2 fallback;
shadcn CLI declined due to custom DESIGN.md token system, hand-rolled cn,
Tailwind v4). Generic DataTable with sort (aria-sort), column show/hide via
hand-styled Radix DropdownMenu (KTD-10 floor), pagination (prev/next +
page-size select, default 25), toolbar slots, opt-in columns (R19), and
DESIGN.md styling. Semi-controlled viewState matches TanStack state shapes
so U2 persistence drops in. Carries R2, R5, R15, R18, R19.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Enriched requirements-only plan to implementation-ready (KTD-1..KTD-11,
U1-U8, verification contract, DoD) and folded in document-review fixes;
records the three review-residual judgment calls (KTD-7 no-flash, U1
first-consumer ordering, U8 split).

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
U2: useTableViewState persists per-table sort/columns/pageSize to
localStorage under the KTD-7 schema (magstacker:table:<id>:v1) via a pure,
bun-tested serialize/parse helper (fail-safe: malformed or wrong-version
entries fall back to defaults, never throw). The shared DataTable gains a
generic `mounted` prop that renders a neutral skeleton until restore
completes, so saved settings apply on the first real paint with no
defaults-then-swap flash (R3, following theme-toggle.tsx).

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…mpiler reactivity

Migrates the admin users table (the simplest flat consumer, RES-2) onto the
shared DataTable wrapper with sort, column show/hide, pagination, and
localStorage persistence, and lands the first e2e/table-view-controls.spec.ts.

As the first real consumer it surfaced a foundation bug: with reactCompiler
enabled, the memoized toolbar controls (Pagination, ColumnMenu) never
re-rendered because their only prop was TanStack's referentially-stable table
object — the compiler cannot see its internal state mutate, so controls showed
stale values while the table body updated. Fix: pass the reactive view-state
slices (columnVisibility, pageIndex, pageSize, rowCount) explicitly so the
compiler re-renders the controls. Also make the column menu non-modal and keep
it open across toggles so column changes are visible live.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ion)

U5: GroupedTableView renders owner-scoped roll-ups — owned rows bucketed into
count-desc/name-asc groups (R13) with keyboard-operable, Motion-animated
expand/collapse (R11/R15/R17, reduced-motion instant fallback) and focus-return
on collapse (KTD-11c), a flat Shared-with-you section for borrowed rows (R10),
and the three grouped empty states (R16). Reuses the shared toolbar (pagination
off, R14) and the wrapper's header/cell styling; the sort bridge maps the active
column sort to member order (R13). Toolbar pagination props made optional for
grouped mode. Consumer + grouping e2e land with U6.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… primitive

Pivots the data-table to idiomatic shadcn/Radix components (user direction) and
completes the migration of every table onto the shared wrapper.

Foundation:
- shadcn init: components.json, clsx+tailwind-merge cn, generated table,
  dropdown-menu, collapsible; lucide-react + tw-animate-css + radix-ui deps.
- Token bridge in globals.css maps shadcn semantic tokens onto the DESIGN.md
  "Machined Console" palette so generated components render on-brand without
  per-component restyling (temporary — tracked in #47).

Grouped view (U5) rebuilt on Radix Collapsible + shadcn Table: each group is a
Collapsible with its own Table, replacing the hand-rolled expand state /
invalid element ids / Motion that hung the click in-browser. Radix owns
open-state, keyboard, focus, and aria; tw-animate-css handles the height
animation and reduced-motion.

Migrations: magazines (U6, client filter + None/By type/By label prefix
grouping, capacity aggregate), firearms (U7, first-class type filter + None/By
type grouping, count-only headers), users, summary's two aggregate tables (U8,
KTD-8 server-aggregated), range-session-history (U8). Old components/ui/table.tsx
primitive replaced by shadcn Table; magazines URL filter retired (KTD-3).

Fix: column defs depend on stable del.request and a ref-read linkLabel so a list
refetch (e.g. a ShareControl grant) doesn't rebuild columns and remount the
actions cell — which was dropping delete-dialog focus restore and ShareControl
dialog state.

e2e: table-grouping.spec.ts (By type roll-up, filter-then-group, persistence,
opt-in columns). just ci-check green (263 unit + 24 e2e).

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
@unclesp1d3r unclesp1d3r self-assigned this Jul 5, 2026
Copilot AI review requested due to automatic review settings July 5, 2026 06:20
@coderabbitai

coderabbitai Bot commented Jul 5, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR adds shadcn/ui integration, a shared TanStack DataTable system with persisted view state and grouping, and migrates the admin, firearms, magazines, summary, and range-session-history views onto the new components. It also adds grouping/storage utilities, Playwright coverage, and a rollout plan.

Changes

DataTable system and table migrations

Layer / File(s) Summary
shadcn setup and shared primitives
.mcp.json, components.json, app/globals.css, components/ui/cn.ts, components/ui/collapsible.tsx, components/ui/dropdown-menu.tsx, components/ui/table.tsx, package.json
Adds shadcn config and MCP wiring, token bridging, updated cn, Radix wrappers, the new table primitive suite, and required dependencies.
Grouping engine and view-state storage
src/domain/tables/grouping.ts, src/domain/tables/magazine-groups.ts, src/domain/tables/firearm-groups.ts, src/domain/tables/view-state-storage.ts, src/domain/tables/__tests__/*
Adds owner-scoped grouping, magazine/firearm key selectors, versioned localStorage serialization, and Bun test coverage.
Shared DataTable controls
components/ui/data-table/types.ts, hooks/use-table-view-state.ts, components/ui/data-table/data-table.tsx, components/ui/data-table/data-table-toolbar.tsx, components/ui/data-table/column-menu.tsx, components/ui/data-table/pagination.tsx
Adds the shared DataTable, skeleton, toolbar, column menu, pagination, and persisted view-state hook.
GroupedTableView roll-up rendering
components/ui/data-table/grouped-table-view.tsx
Adds grouped rendering for owned rows, borrowed rows, collapsible group panels, and shared header/body cell helpers.
Admin users, summary, and range-session-history migrations
app/(admin)/users/admin-users.tsx, app/(app)/summary/page.tsx, app/(app)/summary/summary-tables.tsx, app/(app)/firearms/range-session-history.tsx, e2e/table-view-controls.spec.ts
Migrates the admin users table, the summary roll-up tables, and the range-session-history widget onto the shared DataTable API, with Playwright coverage for the admin view controls.
Firearms and magazines migration
app/(app)/firearms/firearms-view.tsx, app/(app)/magazines/magazines-view.tsx, app/(app)/magazines/page.tsx, app/(app)/magazines/filter-bar.tsx (removed), e2e/table-grouping.spec.ts, e2e/fixtures/user-pool.ts, docs/plans/2026-07-04-002-feat-data-table-view-controls-plan.md
Migrates the firearms and magazines views to client-side view state, grouped rendering, local filtering, and the new shared table controls, with E2E coverage, the seeded spec user update, and the rollout plan.

Possibly related issues

Possibly related PRs

Suggested labels: enhancement, frontend, shared, dependencies

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title matches the change, but it is not valid Conventional Commits because it omits the space after the colon. Change it to feat(ui): Integrate shadcn dependency and enhance shared DataTable features.
Description check ⚠️ Warning The description covers the change, but it omits the required template sections for Summary, Related issue, Changes, Test plan, AI disclosure, and Checklist. Add the missing template sections and fill in the test plan, AI assistance disclosure, and checklist items.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch 36-roll-up-identical-magazine-types-group-by-name-or-label-prefix-on-the-magazines-page

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added dependencies Pull requests that update a dependency file enhancement New feature or request frontend labels Jul 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes table rendering across the app by introducing a shared TanStack-powered DataTable (with column visibility + sorting + pagination), adding per-table view-state persistence in localStorage, and implementing owner-scoped roll-up grouping for magazines/firearms with supporting domain logic and test coverage.

Changes:

  • Added shared DataTable UI primitives (toolbar, column menu, pagination) and a useTableViewState hook for persistent per-table settings.
  • Introduced pure domain grouping + view-state serialization helpers with Bun unit tests, plus Playwright coverage for view controls and grouping.
  • Migrated existing tables/pages (magazines, firearms, users, summary, range session history) to the shared table system and removed the magazines URL-based FilterBar.

Reviewed changes

Copilot reviewed 35 out of 36 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/domain/tables/view-state-storage.ts Adds versioned view-state key + serialize/parse helpers for localStorage persistence.
src/domain/tables/magazine-groups.ts Adds magazine grouping key selectors + capacity aggregation helpers.
src/domain/tables/grouping.ts Adds generic owned/borrowed split + grouping engine.
src/domain/tables/firearm-groups.ts Adds firearm-by-type grouping key selector.
src/domain/tables/tests/view-state-storage.test.ts Unit tests for view-state storage key + parse fail-safe behavior.
src/domain/tables/tests/magazine-groups.test.ts Unit tests covering magazine grouping acceptance examples + aggregates.
src/domain/tables/tests/grouping.test.ts Unit tests for grouping engine ordering/immutability/aggregate.
src/domain/tables/tests/firearm-groups.test.ts Unit tests for firearm grouping + owned/borrowed split.
hooks/use-table-view-state.ts Client hook to restore/persist per-table view state from/to localStorage.
components/ui/data-table/types.ts Shared table types, default view-state creation, and column meta augmentation.
components/ui/data-table/data-table.tsx Flat table wrapper using TanStack sorting/visibility/pagination + skeleton mount-guard.
components/ui/data-table/data-table-toolbar.tsx Shared toolbar (filter/grouping slots + column menu + pagination).
components/ui/data-table/column-menu.tsx Column visibility dropdown enforcing “at least one column visible”.
components/ui/data-table/pagination.tsx Prev/next + page-size select control.
components/ui/data-table/grouped-table-view.tsx Grouped rendering (owned roll-ups + borrowed section) using pure domain grouping.
components/ui/dropdown-menu.tsx Adds dropdown menu UI wrapper (Radix-backed) used by column menu.
components/ui/collapsible.tsx Adds collapsible UI wrapper (Radix-backed) used for grouped expand/collapse.
components/ui/table.tsx Replaces the previous table primitives with new Table* components.
components/ui/cn.ts Switches cn to clsx + tailwind-merge.
app/globals.css Imports tw-animate-css + adds shadcn token bridge variables.
components.json Adds shadcn config/aliases (even though DataTable is headless TanStack).
app/(app)/magazines/page.tsx Removes URL-based filtering and FilterBar; passes filter inputs into the view.
app/(app)/magazines/magazines-view.tsx Migrates magazines to shared DataTable + adds client filtering + grouping modes + persistence.
app/(app)/magazines/filter-bar.tsx Removes the URL-based filter bar component.
app/(app)/firearms/firearms-view.tsx Migrates firearms to shared DataTable + type filter + grouping + persistence.
app/(app)/firearms/range-session-history.tsx Migrates range session history widget to shared DataTable with persisted view state.
app/(admin)/users/admin-users.tsx Migrates admin users table to shared DataTable with persisted view state.
app/(app)/summary/summary-tables.tsx Adds thin client wrapper to render summary aggregates through DataTable + persistence.
app/(app)/summary/page.tsx Switches summary page to use SummaryTables instead of manual table markup.
e2e/table-view-controls.spec.ts Adds e2e coverage for sort/columns/pagination/persistence/accessibility on /users.
e2e/table-grouping.spec.ts Adds e2e coverage for magazines grouping + filter-then-group + persistence.
e2e/fixtures/user-pool.ts Adds a new pooled user key for the grouping spec.
docs/plans/2026-07-04-002-feat-data-table-view-controls-plan.md Adds implementation plan/spec for the shared table feature set.
package.json Adds TanStack Table + shadcn-adjacent dependencies (Radix/CVA/clsx/tw-merge/etc).
.mcp.json Adds MCP server config for shadcn CLI tooling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/domain/tables/view-state-storage.ts
Comment thread src/domain/tables/view-state-storage.ts
Comment thread src/domain/tables/__tests__/view-state-storage.test.ts
Comment thread hooks/use-table-view-state.ts
Comment thread src/domain/tables/grouping.ts
Comment thread components/ui/data-table/grouped-table-view.tsx
Comment thread components/ui/dropdown-menu.tsx
Comment thread components/ui/collapsible.tsx
Comment thread package.json
Comment thread components/ui/data-table/column-menu.tsx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/(app)/magazines/magazines-view.tsx (1)

314-318: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reset magazine filters before flashing a new row. Any active query/caliber/firearm filter can hide the created magazine, so flash(touchedId) can target a row that isn’t rendered. Clear the magazine filters on create, like the firearms view does for type.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(app)/magazines/magazines-view.tsx around lines 314 - 318, The refresh
helper in magazines-view currently flashes the touched row before any active
filters are cleared, so the new magazine may be hidden and not render. Update
the refresh flow to reset the magazine query/caliber/firearm filters when a
magazine is created, then call flash(touchedId) after the filters are cleared,
following the same pattern used in the firearms view for type. Use the existing
refresh function and the related filter state setters in magazines-view to keep
the row visible before flashing.
🧹 Nitpick comments (2)
components/ui/data-table/data-table.tsx (1)

258-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

SortIcon duplicated with grouped-table-view.tsx.

Identical implementation exists in components/ui/data-table/grouped-table-view.tsx (Lines 368-389). Extract to a shared module (e.g. co-locate with types.ts or a new sort-icon.tsx) so future icon/style tweaks stay in one place.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/ui/data-table/data-table.tsx` around lines 258 - 279, The SortIcon
component is duplicated between data-table.tsx and grouped-table-view.tsx, so
keep the implementation in one shared place instead of maintaining two copies.
Extract SortIcon into a shared module near the existing data-table helpers (for
example alongside types.ts or a new sort-icon.tsx), then update both
data-table.tsx and grouped-table-view.tsx to import and use that shared
component so future icon or styling changes stay centralized.
package.json (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the direct @radix-ui/react-dropdown-menu dependency. radix-ui already brings it in, and there are no direct source imports here; keep the lockfile aligned after dropping it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package.json` at line 20, Remove the direct `@radix-ui/react-dropdown-menu`
entry from package.json since the radix-ui package already provides it and there
are no direct imports using it. After deleting the dependency, update the
lockfile so it stays aligned with the package manifest.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/`(app)/magazines/magazines-view.tsx:
- Around line 300-312: The magazine filter state can become stale when persisted
`viewState.filters.caliber` or `viewState.filters.firearm` no longer exist in
the live option lists, causing blank selects and an empty filtered list; update
`magazines-view.tsx` to mirror the firearms view’s `effectiveFilter` and
reset/reconcile pattern. In `MagazinesView`, derive a safe effective
caliber/firearm filter from the current inventory options before applying the
`filtered` memo, and clear invalid persisted values when inventory changes or
after creating a magazine so the new item is not hidden by an outdated filter.

In `@components/ui/collapsible.tsx`:
- Around line 1-33: The Collapsible component file uses React.ComponentProps in
Collapsible, CollapsibleTrigger, and CollapsibleContent without importing the
React types, which will break typecheck. Add the missing type-only React import
at the top of the module, matching the pattern used by the other UI primitives,
so the component prop types resolve correctly.

In `@components/ui/data-table/data-table.tsx`:
- Around line 39-90: The local pagination state in data-table.tsx keeps
pageIndex unchanged even when the underlying row count shrinks, which can leave
the table on an out-of-range empty page. Update the pagination handling in
data-table.tsx around useReactTable and onPaginationChange so pageIndex is reset
or clamped whenever the effective data length changes, using currentViewState
and the local pageIndex state as the key locations. Make sure the fix preserves
existing sorting/visibility behavior while ensuring consumers that use
filterSlot never stay on a stale page after filtering.

In `@components/ui/data-table/grouped-table-view.tsx`:
- Around line 94-96: The Shared with you table is rendering sortable headers
without a real sorting pipeline, so clicks only change the icon state and never
reorder rows. Update borrowedTable to use actual sorting state by wiring sorting
and onSortingChange, and add getSortedRowModel so the row model reflects header
interactions; also ensure defaultMemberSort/effectiveSorting is applied there
instead of being ignored. Keep the fix centered around grouped-table-view.tsx,
especially the borrowedTable setup and renderHeaderCell behavior.
- Around line 47-50: The grouped table view is showing sortable headers without
actually wiring sorting state into the table, so header clicks only change the
indicator. Update the table setup in grouped-table-view so the borrowed table
receives sorting support via the same sorting state, onSortingChange handler,
and getSortedRowModel used by data-table.tsx; if sorting should not apply there,
disable sorting on that section instead. Use the grouped table component and the
borrowed table configuration in grouped-table-view.tsx as the place to make the
change.

In `@components/ui/table.tsx`:
- Around line 55-66: Grouped table rendering is dropping the row-flash behavior
because `GroupedTableView` still uses plain `TableRow` instances without the
existing flash predicate. Update the grouped rendering path to thread
`isRowFlashed` through both the grouped panels and the shared section, using the
relevant row-rendering helpers in `GroupedTableView` and `TableRow` so newly
added or edited rows keep highlighting when grouping is enabled.

In `@e2e/table-grouping.spec.ts`:
- Around line 1-112: The grouping spec only covers a single user and misses the
R10/AE3 shared-item behavior. Add a new step or separate test in
table-grouping.spec.ts that uses a second seeded user to share a magazine, then
verify in GroupedTableView that the owner’s grouped count excludes the borrowed
item while the borrowed row appears under “Shared with you”. Use the existing
authTest flow and locate the UI assertions around “Group by” and grouped
buttons/headers so the new coverage matches the current accessible selectors.

In `@src/domain/tables/view-state-storage.ts`:
- Around line 27-29: `isRecord` currently treats arrays as plain objects, which
lets array-valued state slip through and be spread into defaults in
`loadViewState`. Update the `isRecord` helper in `view-state-storage` to reject
arrays as well as null so only plain object envelopes pass, and keep the
existing `loadViewState` fallback path unchanged for non-record values.

---

Outside diff comments:
In `@app/`(app)/magazines/magazines-view.tsx:
- Around line 314-318: The refresh helper in magazines-view currently flashes
the touched row before any active filters are cleared, so the new magazine may
be hidden and not render. Update the refresh flow to reset the magazine
query/caliber/firearm filters when a magazine is created, then call
flash(touchedId) after the filters are cleared, following the same pattern used
in the firearms view for type. Use the existing refresh function and the related
filter state setters in magazines-view to keep the row visible before flashing.

---

Nitpick comments:
In `@components/ui/data-table/data-table.tsx`:
- Around line 258-279: The SortIcon component is duplicated between
data-table.tsx and grouped-table-view.tsx, so keep the implementation in one
shared place instead of maintaining two copies. Extract SortIcon into a shared
module near the existing data-table helpers (for example alongside types.ts or a
new sort-icon.tsx), then update both data-table.tsx and grouped-table-view.tsx
to import and use that shared component so future icon or styling changes stay
centralized.

In `@package.json`:
- Line 20: Remove the direct `@radix-ui/react-dropdown-menu` entry from
package.json since the radix-ui package already provides it and there are no
direct imports using it. After deleting the dependency, update the lockfile so
it stays aligned with the package manifest.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: eb52f1cd-d9bf-4695-8e73-5510a74d8631

📥 Commits

Reviewing files that changed from the base of the PR and between 4426a25 and c8e5912.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock, !bun.lock
📒 Files selected for processing (35)
  • .mcp.json
  • app/(admin)/users/admin-users.tsx
  • app/(app)/firearms/firearms-view.tsx
  • app/(app)/firearms/range-session-history.tsx
  • app/(app)/magazines/filter-bar.tsx
  • app/(app)/magazines/magazines-view.tsx
  • app/(app)/magazines/page.tsx
  • app/(app)/summary/page.tsx
  • app/(app)/summary/summary-tables.tsx
  • app/globals.css
  • components.json
  • components/ui/cn.ts
  • components/ui/collapsible.tsx
  • components/ui/data-table/column-menu.tsx
  • components/ui/data-table/data-table-toolbar.tsx
  • components/ui/data-table/data-table.tsx
  • components/ui/data-table/grouped-table-view.tsx
  • components/ui/data-table/pagination.tsx
  • components/ui/data-table/types.ts
  • components/ui/dropdown-menu.tsx
  • components/ui/table.tsx
  • docs/plans/2026-07-04-002-feat-data-table-view-controls-plan.md
  • e2e/fixtures/user-pool.ts
  • e2e/table-grouping.spec.ts
  • e2e/table-view-controls.spec.ts
  • hooks/use-table-view-state.ts
  • package.json
  • src/domain/tables/__tests__/firearm-groups.test.ts
  • src/domain/tables/__tests__/grouping.test.ts
  • src/domain/tables/__tests__/magazine-groups.test.ts
  • src/domain/tables/__tests__/view-state-storage.test.ts
  • src/domain/tables/firearm-groups.ts
  • src/domain/tables/grouping.ts
  • src/domain/tables/magazine-groups.ts
  • src/domain/tables/view-state-storage.ts
💤 Files with no reviewable changes (1)
  • app/(app)/magazines/filter-bar.tsx

Comment thread app/(app)/magazines/magazines-view.tsx Outdated
Comment thread components/ui/collapsible.tsx
Comment thread components/ui/data-table/data-table.tsx
Comment thread components/ui/data-table/grouped-table-view.tsx
Comment thread components/ui/data-table/grouped-table-view.tsx
Comment thread components/ui/table.tsx
Comment thread e2e/table-grouping.spec.ts
Comment thread src/domain/tables/view-state-storage.ts
…ray fields

Review follow-up (PR #48): parseViewState did a flat spread of stored state
over defaults, so a nested field (columnVisibility, filters) replaced its
default wholesale — a later opt-in column added to the defaults would ship
visible for returning users (KTD-4 regression) with no version bump. Now merges
one level deep. Guarded to PLAIN objects only: arrays (sorting/SortingState)
are replaced wholesale, never key-spread into { "0": … }. Adds nested-merge
and array-replacement regression tests.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
@coderabbitai coderabbitai Bot added the shared label Jul 5, 2026
…tionability

The table-grouping e2e clicked the Radix Collapsible trigger via pointer, which
timed out on CI (Playwright's pointer 'stability' check stalls when the trigger
re-renders under React Compiler on a contended runner). Switch to focus+Enter —
deterministic and also exercises R15 keyboard operability.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
The prior CI failures were the 30s default test budget running out mid-interaction
on the CI runner (the test does two bulk-adds + grouping + filter + reload +
opt-in columns; it fits 30s locally but not on CI), not a hung page. Give this
heavy stateful test 90s.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…read block

The Radix Collapsible height-animation (animate-collapsible-* driven by
--radix-collapsible-content-height) re-measures the member table via a
ResizeObserver; combined with the table's horizontal-scroll container under
software rendering (headless CI has no GPU) it thrashes and blocks the renderer
main thread on expand — the table-grouping e2e's expand interaction hung there
on CI while passing locally (GPU). Instant reveal keeps the collapse (R11)
robust; R17 motion degrades to instant.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ble overflow

The shadcn <Table> wraps content in a hardcoded overflow-x-auto div. Inside a
Radix Collapsible, that scrollbar toggling makes the content-height
ResizeObserver thrash under headless/software rendering (CI has no GPU),
blocking the main thread on group expand — the e2e's keyboard-driven expand
timed out there on CI while passing locally. Render member rows in a plain
<table> (shadcn sub-components, no wrapper) and move horizontal overflow (R19)
to one container around the whole groups region, which Radix does not measure.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
@coderabbitai coderabbitai Bot added the testing label Jul 5, 2026
…der loop

Root cause of the CI (and, in the single-table form, local) hang on group
expand: GroupedTableView fed fresh `data.filter(...)` arrays to its two
useReactTable instances every render. TanStack sees new data identity, fires its
autoReset logic, schedules a state update, re-renders, gets new arrays again — an
infinite render loop that pegs the renderer main thread (a CPU profile showed it
churning ReactElement creation). It surfaced whenever the component re-rendered
on its own state, i.e. expanding a group. Memoize the owned/borrowed row sets on
[data, ownerId] so the loop can't arm.

Also completes the move to the idiomatic shadcn grouped-table structure: a single
<table> with expandable group-header rows toggled by React state, replacing the
per-group Radix Collapsible (whose ResizeObserver height-measure was a red
herring). One table, one scroll container, valid useId-based ids.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
@coderabbitai coderabbitai Bot removed the testing label Jul 5, 2026
Two review follow-ups (PR #48):
- admin-users columns no longer depend on `pending` (which flips every toggle),
  so flexRender stops remounting every cell and tearing down the just-clicked
  Disable/Enable button. Third instance of the column-instability class the
  recovered react-review flagged; drop the invisible pending re-click guard
  (double-toggle is idempotent), keep the role guard.
- Add shadcn's radius scale (--radius-sm/md/lg/xl) to @theme inline, derived
  from the DESIGN.md radii, with lg mapped to the existing frame radius so
  shadcn components round on-brand with no conflict against the raw
  rounded-[var(--radius-lg)] usages.

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
- view-state-storage: derive key suffix from VIEW_STATE_VERSION; reject array
  envelope state via isPlainObject (Copilot)
- grouping / grouped-table-view: O(n)-push bucketing instead of O(n^2) spread
  (Copilot); disable sorting on the non-sorted 'Shared with you' table so its
  headers don't render misleading sort toggles (CodeRabbit)
- data-table: reset pageIndex when a filter shrinks the row set below the
  current page (CodeRabbit)
- column-menu: coerce Radix CheckedState to a strict boolean (Copilot)
- collapsible: explicit React type import for consistency (CodeRabbit)
- use-table-view-state: guard localStorage.getItem so a throwing read (Safari
  private mode) can't leave the table stuck unmounted (Copilot)
- package.json: drop now-unused @radix-ui/react-dropdown-menu (Copilot)
- magazines-view: ignore a persisted caliber/firearm filter whose option no
  longer exists so it can't silently hide all rows (CodeRabbit)

Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement New feature or request frontend shared

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adopt a shared data-table with view controls and roll-up grouping (magazines + firearms)

2 participants