Repository navigation
Conversation
brmk
force-pushed
the
feat/dashboard-default-column-filters
branch
2 times, most recently
from
September 5, 2026 21:02
23c8323 to
460d9f3
Compare
Adds a `defaultColumnFilters` prop to `ListPage`, letting a route declare column filters that apply until the user configures their own. This is what a list page needs in order to hide rows behind a soft-hide flag by default — archived products, unpublished drafts, out-of-season items — while still letting the user clear the filter to see them. The defaults apply only while the user has no filter state of their own, which means the saved table settings have to tell "has never configured filters on this page" apart from "has cleared every filter". Both used to look identical: the data table reported its filter state on mount, and `ListPage` persisted whatever it was told, so simply visiting a page saved an empty filter state. The data table now reports filter changes only when the filters actually change, so the absence of a saved `columnFilters` entry means the user has never touched them, and an empty array means they cleared them. Reapplying the defaults over a deliberately cleared state is what that distinction prevents. Existing installs already carry the empty entries that mount-time write produced, one for every list page anyone has ever opened, so without a migration the new prop would be inert for exactly the users it is meant for. The user settings now carry a `tableSettingsVersion` stamp, and settings loaded below the current stamp — from localStorage and from the server-side SettingsStore alike — are brought up to it and the stamp written back. A legacy auto-write cannot be told apart from a genuine clear, so both are dropped: a user who really had cleared every filter on a page that declares defaults sees those defaults one more time and clears them once more, which is much less bad than defaults that never apply at all. The migration is a ladder of individually gated steps rather than one block, so that each step runs only against settings stamped below it. Version 1's step drops the empty entries; once a copy carries that stamp an empty `columnFilters` in it is a real user clear, and a later bump must run only its own new step over it. Settings stamped above the current version — what a rolled-back install reads — are left alone rather than migrated and stamped backwards. `ListPage` also holds the data table back until the user settings have resolved, but only on pages that pass both `defaultColumnFilters` and `pageId`. The table seeds its filter state from props once, on mount, so mounting before the server settings arrive would put the default filter chips on screen over a list that was still being fetched unfiltered. `defaultColumnFilters` requires `pageId`, since without it the filters live only in the URL, which has no way to express the difference; in development, setting one without the other logs a warning rather than silently doing nothing.
brmk
force-pushed
the
feat/dashboard-default-column-filters
branch
from
September 5, 2026 21:53
460d9f3 to
4ac67e5
Compare
Member
Author
|
Submitted upstream as vendurehq#5295 (issue vendurehq#5294). |
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.
Description
Adds a
defaultColumnFiltersprop toListPage, so a route can declare column filters that apply until the user configures their own.ListPagealready acceptsdefaultSort,defaultVisibilityanddefaultColumnOrder, but there is no equivalent for filters. The case that needs one is a list page that hides rows behind a soft-hide flag by default — archived products, unpublished drafts, out-of-season items — while still letting the user clear the filter to see them. Today the only way to get that is to patch the dashboard package.The defaults behave exactly as if the user had set them: they render as active filter chips, and can be edited or removed.
The behaviour contract
Making point 3 possible
Point 3 is the interesting one, and it needed a fix below the new prop.
Filters for a page are saved in the user settings under
tableSettings[pageId].columnFilters(providers/user-settings.tsx), the same store that holdscolumnVisibility,columnOrderandpageSize. There is a precedent for how to express a default there:useViewOptionDefaultsresolves the saved column order withuserTableSettings?.columnOrder ?? [...codeDefaults]— the absence of a saved value is what marks the defaults as still applicable, not its emptiness.defaultColumnFiltersfollows the same rule.That rule did not hold for filters, because
DataTablefiredonFilterChangefrom an effect that also ran on mount. Merely visiting a list page therefore reported the initial filter state andListPagepersisted it, so "has never configured filters here" and "has cleared every filter" both ended up as[]. Under those conditions apersisted?.length ? persisted : defaultscheck re-applies the defaults over a state the user deliberately cleared, on every remount — and, since the first visit freezes whatever the defaults were into the user's settings, a route that later changes its defaults can never deliver them.So
DataTablenow reports filter changes only when the filters actually change (columnFiltersEqual, which also replaces a duplicated inlineJSON.stringifycomparison in the same effect). The mount run reports nothing, and the two states become distinguishable:columnFiltersdefaultColumnFilters[][][...]Applying a saved view still reports its filters directly (
DataTableProvider.handleApplyView), including an empty set, which is correct — the user chose that view.Migrating existing saved settings
Stopping the mount-time write fixes new state, but not the state that is already out there. Every user who has ever opened a list page has an empty
columnFiltersentry saved for it, written by the old unguarded effect. Read by the rule above, those entries say "this user cleared every filter here" — so on any existing installation the new prop would be inert for precisely the users it is meant for. It would only ever work for a brand-new account.So the persisted table settings are now versioned.
UserSettingscarries atableSettingsVersionstamp, andmigrateTableSettings()runs on load as a ladder of individually gated steps: settings stamped below the current version run only the steps above their own stamp, and are then stamped current. Version 1's step drops everycolumnFiltersentry that is an empty array, so those pages go back to the "never configured" state and the defaults apply.The gating matters for what comes next. Once a copy is stamped at 1, an empty
columnFiltersin it is a genuine user clear, so a later bump to 2 must not re-run version 1's drop over it — it runs only the new step. And settings stamped above the current version, which is what a rolled-back install reads, are returned untouched rather than re-migrated and stamped backwards.A legacy auto-write is indistinguishable from a genuine clear — that is the whole problem being fixed — so the migration drops both, and there is one visible consequence worth stating plainly: a user who really had cleared every filter on a page that declares defaults sees those defaults one more time, and clears them once more. After that the stamp is in place and the clear sticks. The alternative — keeping the empty entries — is defaults that silently never apply for every existing user, which is the worse failure of the two. On a page with no
defaultColumnFiltersthe migration is invisible either way.Both persistence paths are migrated, because either one alone would be undone by the other. The settings provider seeds from
localStorageand then, when the server-side SettingsStore resolves, replaces the settings wholesale with the server's copy. Migrating only the local copy would mean the server's old-shape payload landed on top a moment later and resurrected the entries just dropped. So the incoming server payload is migrated too, andserverSettingsis left holding the pre-migration value, which makes the existing save effect push the migrated settings and the stamp back up to the server — the migration runs once per user, not once per session.Asynchronous settings and the mount gate
DataTableseeds its filter state from props once, on mount. The user settings resolve asynchronously, so a table that mounts before the server-side SettingsStore responds keeps filter state that the arriving settings may contradict. That divergence between the chips and the query is pre-existing, and on pages without defaults it is invisible in practice; with defaults it is not, because the default chips would sit on screen while the list query ran unfiltered.The settings provider already exposes
settingsReadyfor exactly this, with a docstring telling one-shot consumers to wait for it.ListPagenow does: it defers mounting the data table untilsettingsReady, only when bothdefaultColumnFiltersandpageIdare set. Scoping the gate to the new feature means no existing list page changes how it mounts. A narrower prop→state sync effect insideDataTablewas considered instead and rejected:columnFiltersarrives as a fresh array on every render, so the effect would also fire on the re-render caused by the user's own edit — at which point the prop still holds the pre-persistence value and the edit would be reverted under them.While the gate is closed the block renders nothing. A loading skeleton would read better than an empty block, and
DataTablealready has one — but it is driven byisLoadingon a mounted table, and mounting the table is precisely what the gate exists to prevent, so using it would mean duplicating the skeleton markup inListPage. Left minimal on purpose; happy to add one if you would prefer it.The residual limitation is unchanged and left as it was: on a list page without
defaultColumnFilters, a table that mounts before the server settings arrive still keeps whatever filter state it was seeded with. Fixing that generally means reconciling props intoDataTable's state, which is a larger change than this PR should carry.Breaking changes
None. The new prop is optional, and pages that do not use it behave as before.
Two behavioural changes beyond the new prop:
columnFiltersentry into the user's saved table settings.columnFiltersentries already in a user's saved settings are dropped once, as described above. Every other saved table setting — visibility, order, page size, non-empty filters — is untouched.Two operational notes on that one-time migration:
setSettingsStoreValuewrites as admins come back online, spread over however long it takes them to log in. It is one small write per user, and nothing repeats it.columnFilters: []back for its page alongside the already-current stamp — which then reads as a genuine clear, permanently exempting that page from defaults for that user until they set and clear a filter there again. It needs a stale tab that writes settings after another session has migrated, so it is narrow, but it is real and it is not detectable after the fact.onColumnVisibilityChangeandonPageChangeemit on mount in the same wayonFilterChangeused to, and are deliberately left alone here. Neither currently has a consumer that needs to distinguish "never set" from "set to the default", so changing them would be scope this PR does not need; the same guard would apply if that changes.Checklist
📌 Always:
👍 Most of the time:
Tests
Unit (
packages/dashboard,vitest run) — full suite green, 734 passed:src/lib/framework/page/list-page.spec.tsx— the four contract points: defaults applied with no saved filters, defaults applied when the page has other saved table settings but no filters, saved filters preferred over defaults, defaults not re-applied over a saved[], and defaults ignored without apageId. Plus the mount gate: the table is not mounted untilsettingsReady, and pages withoutdefaultColumnFiltersor withoutpageIdmount regardless.src/lib/providers/user-settings.spec.tsx— the migration: a legacy emptycolumnFiltersentry is dropped, a non-empty one is preserved, other table settings are untouched, settings stamped below the current version are migrated, already-stamped settings are returned unchanged (so a genuine post-upgrade clear survives), settings stamped above the current version are returned by reference and neither drained nor re-stamped, and, through the provider, both thelocalStoragecopy and the server payload are migrated and the migrated settings are written back to the server.src/lib/components/data-table/data-table-column-filters.spec.ts—columnFiltersEqual: a re-rendered copy of the same filters is not a change, clearing all filters is.E2E (
e2e/tests/components/list-page-filter-persistence.spec.ts, run against the products list):Each half of the change is covered by tests that fail without it: the "cleared" unit test fails against a
persisted?.length ? persisted : defaultsmerge; seven of the elevenuser-settingstests fail with the migration stubbed out, two of them fail if only the server payload is left un-migrated, and the "stamped above the current version" test fails against a migration gated on!==rather than<; the mount-gate test fails without the gate; and the "visiting saves nothing" e2e fails without theDataTablemount fix. The existing filter e2e suites (catalog/product-list.spec.ts,regression/issue-4730-*, 20 tests) pass unchanged.columnFiltersEquallives in its own module rather than indata-table-utils.ts, which is re-exported from the package barrel — it is an internal detail of theDataTable, not new public API.data-table-pagination-state.tsis the existing precedent for a package-internal helper module with its own spec.The e2e specs carry a
// #TBDplaceholder in place of the issue number, per the// #<issue> — descriptionconvention — happy to fill in the issue number this PR gets linked to.Draft upstream issue
ListPagehas no way to declare default column filtersIs your feature request related to a problem? Please describe.
ListPagelets a route declaredefaultSort,defaultVisibilityanddefaultColumnOrder, but nothing for column filters. That leaves no hook for the common "hide these rows unless the user asks for them" case.Our concrete instance: a production store where products carry an
isArchivedcustom field. The products list should hide archived products by default, but an admin needs to be able to clear the filter and see them — archived products still get edited, and they still show up in old orders. There is no way to express that through the public API, so we patch@vendure/dashboarditself in every deploy.It generalises to any plugin that adds a soft-hide flag: draft articles, out-of-season items, deactivated records. Each of them wants its list page to open in a filtered state that the user can escape.
Describe the solution you'd like
A
defaultColumnFilters?: ColumnFiltersStateprop onListPage, applied only while the user has no filter state of their own, and behaving like user-set filters once applied — visible as chips, editable, clearable.The part that needs care is what "no filter state of their own" means. Filters persist to
tableSettings[pageId].columnFilters, and a user who clears every filter has made a deliberate choice that must not be overwritten by the defaults on the next visit. That requires "never configured" and "cleared" to be distinguishable in the saved settings, which they currently are not —DataTablereports its filter state on mount, so visiting a page is enough to save an empty one.That also makes it a change that cannot land on its own: every existing user already has empty entries saved for every list page they have visited, so the saved settings need a one-time migration to drop them, otherwise the feature is inert on any installation that is not brand new.
Describe alternatives you've considered
Filtering in
transformVariablesinstead. That works for the "always hidden" case but not this one, since the user can never turn it off — the filter is invisible to the UI, so there is no chip to clear.Patching
@vendure/dashboard, which is what we do now and would rather not.