Repository navigation
fix(pivot): four axis-filter bugs found in review - #15
Merged
Merged
Conversation
1. React: switching datasets with an invalid config reset the pivot axes but not fieldFilters, so exclusions from the previous dataset silently filtered the next one and were persisted under its storage key. Clear filters in that branch, mirroring Vue's clearConfig(). 2. Filtering out every value rendered the generic "Your pivot table will appear here" placeholder, indistinguishable from an unconfigured pivot. Both skeletons now say the rows are hidden by axis filters when filters are active and the result is empty. 3. The axis filter panel mounted one checkbox per unique field value with no cap, freezing the tab on high-cardinality columns. Rendering is now capped at 200 values with a "N more — search to narrow the list" hint; the All/None quick actions still act on the full value set. 4. drillthrough.ts kept a private copy of the value-stringification contract that packages/core exports as stringifyPivotValue; the two could silently diverge and desync drill-through matching from pivot key building. Use the shared helper. Verified: lint, build, type-check, and unit tests (241/241) all pass; existing blank-handling drill-through tests cover the contract in (4).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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
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
Fixes the four bugs in the axis value filter feature (#10 /
6baada5) surfaced by a code review of that diff.Changes
React leaked filters across datasets (
packages/react/src/hooks/usePivotTable.ts) — switching thedataprop to a dataset with no saved config reset the pivot axes but notfieldFilters, so exclusions from the previous dataset silently filtered the new one and were persisted under its storage key. The invalid-config branch now clears filters, mirroring Vue'sclearConfig()."Exclude all" looked like "not configured" (both
PivotSkeletons) — filtering out every row madecomputePivotResultreturnnull, which rendered the generic "Your pivot table will appear here" placeholder with no hint that filters were the cause. When filters are active and the result is empty, the placeholder now says "All rows are hidden by axis filters — adjust or clear the filters to see results". Plumbed via a new optionalhasActiveFieldFiltersprop.Unbounded filter panel (both
PivotConfigs) — the panel mounted one checkbox per unique field value, freezing the tab on high-cardinality columns (IDs, timestamps). Rendering is capped at 200 values with a "N more — search to narrow the list" hint; the All/None quick actions still operate on the full value set.Duplicate stringification contract (
packages/core/src/pivot/drillthrough.ts) — a privatestringifyFieldValueduplicated the exportedstringifyPivotValue; divergence would desync drill-through row matching from pivot key building. Deleted in favor of the shared helper.Verification
pnpm lint,pnpm build,pnpm type-checkall cleanpnpm test:unit— 241/241 passing; the existing blank-handling drill-through tests (tests/unit/pivot.test.ts) lock the stringification contract touched in (4)Generated by Claude Code