Open
Conversation
Contributor
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
Contributor
📦 Alpha Package Version PublishedUse Use |
Contributor
🔍 Visual review for your branch is published 🔍Here are the links to: |
Contributor
…ider deps - FiltersControls: skip localFiltersValue keys absent from filter definitions, preventing crashes when switching between visualizations with different schemas - useDataCollectionStorage: move setStorageReady(true) inside the async .then() callback so storage reports readiness only after values are actually restored - LaneProvider: destructure hook return into stable fields for useEffect deps, breaking an infinite render loop caused by unstable object identity
Define optional per-visualization filter and preset overrides on visualization configs. Each visualization type in the union now intersects with VisualizationFilterOverrides<Filters>, allowing views to declare their own `filters` (subset of source filters) and/or `presets` (replacing source presets for that view). Export the new type from the public barrel for consumer use.
Core hook that manages independent filter state per visualization. When no visualization declares overrides it is a pure pass-through. Otherwise each view gets its own currentFilters with save/restore on switch, synchronous transition bridging via pendingFiltersRef, and idempotent storage initialization. Includes 16 test cases covering: pass-through mode, effective filters/presets resolution, save-restore on switch, storage restore, init idempotency, synchronous transitions, and default preset fallback for never-visited views.
Wire usePerVisualizationFilters into OneDatacollection.tsx: - Patch source with effectiveSource so children see correct filters - Use effectiveFilters/effectivePresets for OneFilterPicker rendering - Use activeCurrentFilters/activeSetCurrentFilters for state management - Persist allVisualizationFilters through useDataCollectionStorage - Include currentVisualization in filter/preset change event emissions Storage types extended with visualizationFilters feature for persisting per-view filter state across page reloads.
Three stories demonstrating usage patterns: - FiltersPerVisualization: table (department only) vs card (salary+search) - MixedGlobalAndPerViewFilters: table overrides, card inherits global - PerViewPresetsOnly: table with custom presets, list with global
xavikp
force-pushed
the
add-multifilter-datacollection
branch
from
ffb452c to
6985ec9
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds per-visualization filter and preset overrides to OneDataCollection, enabling each visualization (table, card, list, kanban) to declare its own independent filter configuration while maintaining separate filter states across view switches.
Changes:
- Added
VisualizationFilterOverridestype allowing visualizations to declare optionalfiltersandpresetsproperties - Implemented
usePerVisualizationFiltershook managing independent filter state per visualization with synchronous transitions viapendingFiltersRef - Extended storage system to persist per-visualization filter states under
visualizationFilterskey
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
visualizations/collection/types.ts |
Adds VisualizationFilterOverrides type and applies it to all visualization union variants |
hooks/usePerVisualizationFilters.ts |
Core hook implementing filter resolution, state save/restore, and synchronous transition logic |
OneDatacollection.tsx |
Integrates per-viz filters hook, patches source with effective filters, passes visualization index to event emitter |
useEventEmitter.ts |
Adds optional currentVisualization parameter to include viz index in filter/preset events |
hooks/useDataColectionStorage/types.ts |
Extends storage types with visualizationFilters feature |
hooks/useDataColectionStorage/useDataCollectionStorage.ts |
Fixes storage ready timing - sets flag after restoration completes |
hooks/useDataCollectionData/useDataCollectionLanesData.tsx |
Improves dependency tracking by destructuring hook values for useEffect |
components/OneFilterPicker/components/FiltersControls.tsx |
Adds defensive check for missing filter keys when switching visualizations |
hooks/__tests__/usePerVisualizationFilters.test.ts |
Comprehensive test suite with 16 test cases covering all behavior scenarios |
__stories__/filters/per-visualization-filters.stories.tsx |
Three Storybook examples demonstrating usage patterns |
exports.ts |
Exports VisualizationFilterOverrides type for public API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+38
to
+39
| /** Additional presets shown only when this visualization is active. | ||
| * These are displayed alongside the global source presets. */ |
There was a problem hiding this comment.
The documentation states "These are displayed alongside the global source presets" but the actual implementation at lines 211-212 shows that visualization presets completely replace source presets, not merge with them. The documentation should be corrected to say "These replace the global source presets for this visualization" to match the actual behavior.
Suggested change
| /** Additional presets shown only when this visualization is active. | |
| * These are displayed alongside the global source presets. */ | |
| /** Preset configuration used only when this visualization is active. | |
| * These replace the global source presets for this visualization. */ |
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
1 participant
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.
Description
Add per-visualization filter and preset overrides to
OneDataCollection. Each visualization can declare its ownfiltersand/orpresets, letting different views show only the relevant filter controls while maintaining independent filter state across view switches.Problem and constraints
OneDataCollectionsupports multiple visualization types (table, card, list, kanban) but shares a single filter/preset set across all of them. Some views need only a subset of filters, and switching views should not lose the user's applied filters.Key constraints:
setCurrentFiltersand the parent re-rendering with new props, child components must see the target view's filters synchronously (not stale filters from the previous view)setAllVisualizationFiltersonly takes effect onceBehavior rules
effectiveFilterseffectivePresetssource.filterssource.presetsfiltersfilterspresetspresets(replaces source, not merged)filterspresetssource.filterssource.presetsState transition guarantees
useLayoutEffectoncurrentVisualizationchange saves outgoing state tofilterStatesRef, restores incoming state viasourceSetCurrentFilterspendingFiltersRefholds the target state between the layout effect call and the next render, socurrentFiltersreturns the correct value immediatelyappliedInitRefensuressetAllVisualizationFilters(called by storage) only applies once{}Backward compatibility
hasPerVisualizationFiltersisfalsewhen no visualization declaresfiltersorpresets→ all new code paths are skippedeffectiveSourcepasses throughsourceunchanged when the feature is offallVisualizationFiltersis{}when off, so existing storage keys are unaffectedReviewer guide
Suggested reading order:
visualizations/collection/types.ts— newfiltersandpresetsfields on visualization configusePerVisualizationFilters.ts— core hook (filter resolution, state save/restore, synchronous bridge)OneDatacollection.tsx— integration point (effectiveSourcepatching)useDataCollectionStorage.ts— persistence ofallVisualizationFiltersusePerVisualizationFilters.test.ts— 16 test cases covering all behavior rulesper-visualization-filters.stories.tsx— 3 Storybook stories demonstrating usagemultiPresetsFilters.mp4