Skip to content

fix(data-table): update table body immediately when toggling columns#6253

Merged
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
DawnMoon1542:fix/data-table-column-visibility-rerender
Jul 17, 2026
Merged

fix(data-table): update table body immediately when toggling columns#6253
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
DawnMoon1542:fix/data-table-column-visibility-rerender

Conversation

@DawnMoon1542

@DawnMoon1542 DawnMoon1542 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

⚠️ PR Notice

Important

  • This PR is AI-assisted. The root cause, fix scope, and typecheck results were reviewed before submission.

📝 Description

On list pages such as Channels, using View → Toggle columns updates the header, but the table body does not follow right away. A full refresh, or another action that rebuilds column definitions (for example the sensitive-data visibility button on Channels), is needed before the body matches the selected columns.

This is the same class of bug as #5524 (row selection not updating). DataTableRow uses a custom React.memo comparator, and TanStack Table row objects keep a stable reference when columnVisibility changes. Without a visible-column dependency, the memo skips the update and getVisibleCells() does not run again.

Fix, aligned with the isSelected snapshot in #5524:

  • In the outer DataTableRow, build a visibleColumnIds signature from row.getVisibleCells() on each render
  • Pass that signature into the memoized inner row and include it in the comparator
  • When the visible column set or order changes, the signature changes and the body re-renders immediately

Change is limited to the shared data-table-row.tsx. Every table that renders through DataTableRow (Channels, API Keys, Users, usage logs custom renderRow, etc.) benefits; feature-specific table logic is unchanged.

🚀 Type of change

  • 🐛 Bug fix - Link a related Issue when possible; do not classify design trade-offs or expectation mismatches as bugs
  • ✨ New feature - Large features should be discussed in an Issue first
  • ⚡ Performance / refactor
  • 📝 Documentation

🔗 Related Issue

✅ Checklist

  • Human-reviewed: Description was written and checked; not a raw dump of unedited AI output.
  • Not a duplicate: Searched Issues and PRs; no open item for this bug. Related history: merged fix(data-table): capture row selection state in memo comparison #5524 (row selection memo), closed fix(web): persist channels table view options #5363 (persist column visibility)—different from "body not refreshing after toggle".
  • Bug fix note: Real render bug from a missing memo dependency, not a design trade-off or misunderstood expectation.
  • Understood: Change behavior and impact are understood.
  • Focused scope: No unrelated code changes.
  • Local verification: Typecheck passed; oxlint on the touched file reported no new issues.
  • Security: No secrets; follows project conventions.

📸 Proof of Work

Local checks:

cd web/default
bun run typecheck
bunx oxlint -c .oxlintrc.json src/components/data-table/core/data-table-row.tsx
  • bun run typecheck passed
  • oxlint on the target file clean

Suggested manual check:

  1. Open Channels in table view
  2. View → Toggle columns and show/hide several columns
  3. Table body should match the header immediately, without refresh or the sensitive-data toggle
  4. Other tables with View options (API Keys, usage logs) should behave the same

Summary by CodeRabbit

  • Bug Fixes
    • Updated data table rows to refresh correctly when visible columns change.
    • Ensured column visibility updates are reflected immediately, even when row data remains unchanged.

DataTableRow's React.memo did not depend on columnVisibility.
TanStack row references stay stable when visible columns change, so the
table body did not refresh after View -> Toggle columns until another
action rebuilt the column definitions.

Capture a visible column id signature (visibleColumnIds) outside the
memo and compare it, matching the existing isSelected snapshot pattern.
All tables that use DataTableRow pick up the fix.
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 40c36024-d512-4b1e-9f3d-98bb74405381

📥 Commits

Reviewing files that changed from the base of the PR and between a63364d and 6db1076.

📒 Files selected for processing (1)
  • web/default/src/components/data-table/core/data-table-row.tsx

Walkthrough

DataTableRow now snapshots visible leaf-column IDs and passes that signature into the memoized inner row. The comparator checks the signature so column-visibility changes rerender rows even when TanStack row references remain stable.

Changes

Data table row visibility

Layer / File(s) Summary
Visible-column memo snapshot
web/default/src/components/data-table/core/data-table-row.tsx
DataTableRow derives and passes a joined visible-column ID signature, while DataTableRowInner accepts it without rendering it as a DOM prop. The memo comparator now compares the signature and documents the explicit state snapshot behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: calcium-ion

Poem

I’m a rabbit watching columns align,
Snapshotting leaves in a tidy little line.
When views change shape, rows wake anew,
Memoized magic now sees it too. 🐇

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implements visible-column rerendering, but it does not satisfy #5524's selection-state fix or #5363's persistence behavior. Add the selection-state snapshot/comparison fix for #5524 and the column-visibility persistence/reload logic for #5363, or unlink those issues if they are reference-only.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: making the data table body update immediately when columns are toggled.
Out of Scope Changes check ✅ Passed All code changes are confined to the data table row component and align with the stated rerendering fix.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Calcium-Ion
Calcium-Ion merged commit dc9aeab into QuantumNous:main Jul 17, 2026
2 checks passed
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.

2 participants