Skip to content

fix: DataTable reset and zero state handling#666

Open
rsbh wants to merge 3 commits intomainfrom
fix_data_table_reset_click
Open

fix: DataTable reset and zero state handling#666
rsbh wants to merge 3 commits intomainfrom
fix_data_table_reset_click

Conversation

@rsbh
Copy link
Member

@rsbh rsbh commented Mar 5, 2026

Summary

  • Reset to default now correctly resets sort and grouping by memoizing defaultTableQuery and explicitly setting sort/group_by to defaults in onDisplaySettingsReset
  • Zero state vs empty state now accounts for sort/group changes via new hasActiveQuery util, preventing incorrect zero state when sort or grouping is modified
  • Added defaultSort to all existing tests missing it

Test plan

  • Reset click resets sort and group to defaults (server mode)
  • Sort/group change in client mode does not show zero state
  • Empty state shown when sort is changed and no data
  • Empty state shown when group is changed and no data
  • All 160 existing + new tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved Display Settings Reset functionality to properly reset sort and grouping to their default values.
  • Improvements

    • Enhanced zero-state and empty-state handling to better distinguish between no data with no active filters versus no data with active filters applied.

rsbh and others added 2 commits March 5, 2026 15:33
Memoize defaultTableQuery and explicitly reset sort/group_by in
onDisplaySettingsReset so reset always restores defaultSort and
no grouping, regardless of initial query prop.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add hasActiveQuery util to check for active filters, search, or
non-default sort/grouping. Use it in content and virtualized-content
to correctly distinguish zero state from empty state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
apsara Ready Ready Preview, Comment Mar 5, 2026 10:50am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2026

📝 Walkthrough

Walkthrough

This PR refactors the data-table component's zero-state and empty-state detection logic by introducing a new hasActiveQuery utility function. The utility consolidates checks for active filters, search terms, and non-default sort/grouping configurations. Components and tests are updated to use this new utility and properly handle defaultSort parameter throughout the component tree.

Changes

Cohort / File(s) Summary
New Utility Function
packages/raystack/components/data-table/utils/index.tsx
Introduced hasActiveQuery utility that evaluates whether a table query is active by checking filters, search, and deviations from default sort/grouping configurations.
Main Component Logic
packages/raystack/components/data-table/data-table.tsx
Added defaultSort handling via useMemo, enhanced onDisplaySettingsReset to properly merge state with default values, and updated state setters to use functional update patterns for consistency.
Rendering Components
packages/raystack/components/data-table/components/content.tsx, packages/raystack/components/data-table/components/virtualized-content.tsx
Replaced explicit filter/search presence checks with hasActiveQuery to determine zero/empty state conditions; isZeroState triggers when data is absent and no changes exist, isEmptyState when data is absent but changes are present.
Test Suite
packages/raystack/components/data-table/__tests__/data-table.test.tsx
Added IntersectionObserver mocking, new "Display Settings Reset" test suite covering server and client modes with popover interactions and reset behavior validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • rohanchkrabrty

Poem

🐰 When defaults align with queries true,
States empty, zero—now I know what's new!
The table resets with a gentle touch,
Sorting and grouping matter so much.
A hop, skip, and jump through the refactor dance! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main changes: fixing DataTable reset behavior and zero state handling, which aligns with the core objectives of the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_data_table_reset_click

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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 and usage tips.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/raystack/components/data-table/__tests__/data-table.test.tsx`:
- Around line 3-14: The test suite replaces global.IntersectionObserver in
beforeAll but doesn't restore it; capture the original value (e.g., const
_origIntersectionObserver = global.IntersectionObserver) before mocking and
restore it in an afterAll teardown by setting global.IntersectionObserver =
_origIntersectionObserver, keeping the mock implementation created with
vi.fn().mockImplementation intact and referencing beforeAll and afterAll in the
test file to avoid leaking the mock to other tests.

In `@packages/raystack/components/data-table/utils/index.tsx`:
- Around line 170-171: The current hasFilters computation only checks
tableQuery.filters.length which treats empty filter rows as active; update the
logic used by hasFilters (and any dependent hasActiveQuery) to detect only
filters with usable values by verifying at least one entry in tableQuery.filters
has a non-null, non-empty value (handle empty strings, empty arrays, and
null/undefined) rather than just checking array length; locate and modify the
hasFilters assignment in index.tsx to use Array.prototype.some on
tableQuery.filters to determine true activity.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c3da4730-8ccd-4185-a970-ec067f98539d

📥 Commits

Reviewing files that changed from the base of the PR and between 1301b55 and 5f19a9e.

📒 Files selected for processing (5)
  • packages/raystack/components/data-table/__tests__/data-table.test.tsx
  • packages/raystack/components/data-table/components/content.tsx
  • packages/raystack/components/data-table/components/virtualized-content.tsx
  • packages/raystack/components/data-table/data-table.tsx
  • packages/raystack/components/data-table/utils/index.tsx

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