diff --git a/web/app/page.tsx b/web/app/page.tsx index 4d7744b..0f4206e 100644 --- a/web/app/page.tsx +++ b/web/app/page.tsx @@ -63,8 +63,8 @@ async function HomeContent({ initialFormats, initialGroupFilter, }: { - initialEngines: string[]; - initialFormats: string[]; + initialEngines: string[] | null; + initialFormats: string[] | null; initialGroupFilter: GroupFilter | null; }) { const [groups, universe] = await Promise.all([cachedGroups(), cachedFilterUniverse()]); diff --git a/web/components/FilterBar.test.tsx b/web/components/FilterBar.test.tsx new file mode 100644 index 0000000..db57a29 --- /dev/null +++ b/web/components/FilterBar.test.tsx @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +// @vitest-environment jsdom + +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { FilterBar } from '@/components/FilterBar'; +import { parseFilterCsv, seriesPassesFilter } from '@/lib/chart-format'; +import { getGlobalFilterSnapshot } from '@/lib/chart-store'; + +const universe = { engines: ['duckdb', 'datafusion'], formats: ['vortex', 'parquet', 'lance'] }; + +describe('global filter URL round trips', () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + }); + + async function mount(): Promise { + const params = new URL(window.location.href).searchParams; + root = createRoot(container); + await act(async () => { + root.render( + , + ); + }); + } + + async function click(dim: string, value: string): Promise { + await act(async () => { + container + .querySelector(`[data-filter="${dim}"][data-value="${value}"]`)! + .click(); + }); + } + + async function reload(): Promise { + await act(async () => root.unmount()); + await mount(); + } + + it.each(['/', '/chart/qm.example'])( + 'preserves none and all formats after reload on %s', + async (path) => { + window.history.replaceState( + null, + '', + `${path}?n=all&group=random-access&hide=duckdb&show=lance#random-access`, + ); + await mount(); + expect(getGlobalFilterSnapshot().active.formats).toEqual(['vortex', 'parquet']); + + for (const engine of universe.engines) await click('engine', engine); + for (const format of ['vortex', 'parquet']) await click('format', format); + let url = new URL(window.location.href); + expect(url.searchParams.get('engine')).toBe(''); + expect(url.searchParams.get('format')).toBe(''); + await reload(); + expect(getGlobalFilterSnapshot().active).toEqual({ engines: [], formats: [] }); + + await click('engine', '*'); + await click('format', '*'); + url = new URL(window.location.href); + expect(url.searchParams.has('engine')).toBe(false); + expect(url.searchParams.get('format')).toBe('vortex,parquet,lance'); + expect(url.searchParams.get('n')).toBe('all'); + expect(url.searchParams.get('group')).toBe('random-access'); + expect(url.searchParams.get('hide')).toBe('duckdb'); + expect(url.searchParams.get('show')).toBe('lance'); + expect(url.hash).toBe('#random-access'); + expect(url.pathname).toBe(path); + await reload(); + expect(getGlobalFilterSnapshot().active).toEqual(universe); + }, + ); + + it('keeps stale explicit allowlists filtered even when their length matches the universe', async () => { + window.history.replaceState(null, '', '/?engine=duckdb,gone&format=vortex,old,older'); + await mount(); + const { active } = getGlobalFilterSnapshot(); + expect(seriesPassesFilter({ engine: 'datafusion', format: 'vortex' }, active, universe)).toBe( + false, + ); + expect(seriesPassesFilter({ engine: 'duckdb', format: 'lance' }, active, universe)).toBe(false); + expect(container.querySelector('[data-role="filter-badge"]')?.textContent).toBe('3'); + await click('engine', '*'); + expect(new URL(window.location.href).searchParams.get('format')).toBe('vortex,old,older'); + await reload(); + expect(getGlobalFilterSnapshot().active.formats).toEqual(['vortex', 'old', 'older']); + }); +}); diff --git a/web/components/FilterBar.tsx b/web/components/FilterBar.tsx index e12a6d9..0052f50 100644 --- a/web/components/FilterBar.tsx +++ b/web/components/FilterBar.tsx @@ -31,7 +31,7 @@ import { * indicator. Every change re-paints the chips (via the store subscription, here * and in every chart island) and syncs the URL `?engine=`/`?format=` allowlists * with `history.replaceState`, so a refresh or share preserves the view; the - * params are omitted when a row is fully active so the no-filter URL is clean. + * params are omitted only when the active set matches the defaults. */ export function FilterBar({ universe, @@ -39,10 +39,10 @@ export function FilterBar({ initialFormats, }: { universe: FilterUniverse; - /** URL `?engine=` allowlist parsed server-side; empty means no filter. */ - initialEngines: string[]; - /** URL `?format=` allowlist parsed server-side; empty means no filter. */ - initialFormats: string[]; + /** URL `?engine=` allowlist parsed server-side; null uses defaults; empty hides all. */ + initialEngines: string[] | null; + /** URL `?format=` allowlist parsed server-side; null uses defaults; empty hides all. */ + initialFormats: string[] | null; }) { const [open, setOpen] = useState(false); const barRef = useRef(null); @@ -101,8 +101,8 @@ export function FilterBar({ }, [open]); const hiddenCount = - Math.max(0, universe.engines.length - activeEngines.length) + - Math.max(0, universe.formats.length - activeFormats.length); + universe.engines.filter((engine) => !activeEngines.includes(engine)).length + + universe.formats.filter((format) => !activeFormats.includes(format)).length; const onChipClick = (dim: 'engine' | 'format', value: string): void => { toggleGlobalFilterValue(dim, value); @@ -204,8 +204,8 @@ function FilterRow({ /** * Mirror the active filter onto the URL as `?engine=`/`?format=` allowlists via - * `history.replaceState`. A param is emitted only when its active set is a - * strict subset of the universe; an all-active row leaves the URL clean. + * `history.replaceState`. Only default selections omit their parameter. An + * empty selection writes an empty value, and all formats includes Lance. */ function syncFilterUrl(): void { if (!window.history?.replaceState) { @@ -214,12 +214,12 @@ function syncFilterUrl(): void { const { universe, active } = getGlobalFilterSnapshot(); const url = new URL(window.location.href); syncDimensionUrl(url, 'engine', active.engines, universe.engines); - syncDimensionUrl(url, 'format', active.formats, universe.formats); + syncDimensionUrl(url, 'format', active.formats, seedActiveFormats(null, universe.formats)); window.history.replaceState(null, '', url.toString()); } -function syncDimensionUrl(url: URL, paramName: string, active: string[], universe: string[]): void { - if (active.length < universe.length) { +function syncDimensionUrl(url: URL, paramName: string, active: string[], defaults: string[]): void { + if (active.length !== defaults.length || !defaults.every((value) => active.includes(value))) { url.searchParams.set(paramName, active.join(',')); } else { url.searchParams.delete(paramName); diff --git a/web/components/Header.tsx b/web/components/Header.tsx index 4c11017..e14d925 100644 --- a/web/components/Header.tsx +++ b/web/components/Header.tsx @@ -37,8 +37,8 @@ export function Header({ initialFormats, }: { universe?: FilterUniverse; - initialEngines?: string[]; - initialFormats?: string[]; + initialEngines?: string[] | null; + initialFormats?: string[] | null; }) { const [navOpen, setNavOpen] = useState(false); const [nextTheme, setNextTheme] = useState<'light' | 'dark'>('light'); @@ -159,8 +159,8 @@ export function Header({ {showFilters && ( )} {/* Mobile-only GitHub link rendered inside the hamburger panel; diff --git a/web/lib/chart-format.test.ts b/web/lib/chart-format.test.ts index d5254e2..359ade2 100644 --- a/web/lib/chart-format.test.ts +++ b/web/lib/chart-format.test.ts @@ -576,12 +576,13 @@ describe('filter helpers', () => { it('parses CSV allowlists with trimming and dedupe', () => { expect(parseFilterCsv('duckdb, datafusion,duckdb,,')).toEqual(['duckdb', 'datafusion']); - expect(parseFilterCsv(null)).toEqual([]); + expect(parseFilterCsv(null)).toBeNull(); + expect(parseFilterCsv(undefined)).toBeNull(); expect(parseFilterCsv('')).toEqual([]); }); it('seeds the active set from the allowlist or the whole universe', () => { - expect(seedActiveFromAllowlist([], universe.engines)).toEqual(['duckdb', 'datafusion']); + expect(seedActiveFromAllowlist(null, universe.engines)).toEqual(['duckdb', 'datafusion']); // A non-empty allowlist is verbatim, even when stale against the universe. expect(seedActiveFromAllowlist(['gone'], universe.engines)).toEqual(['gone']); }); @@ -589,11 +590,11 @@ describe('filter helpers', () => { it('seeds formats with lance hidden by default, but honors an explicit allowlist', () => { const formats = ['vortex', 'parquet', 'lance']; // No allowlist: lance is dropped from the default active set. - expect(seedActiveFormats([], formats)).toEqual(['vortex', 'parquet']); + expect(seedActiveFormats(null, formats)).toEqual(['vortex', 'parquet']); // An explicit allowlist is verbatim, so a `?format=` URL can pin lance on. expect(seedActiveFormats(['lance'], formats)).toEqual(['lance']); // A universe without lance is unaffected. - expect(seedActiveFormats([], ['vortex', 'parquet'])).toEqual(['vortex', 'parquet']); + expect(seedActiveFormats(null, ['vortex', 'parquet'])).toEqual(['vortex', 'parquet']); }); it('hides a series only when its own dimension is filtered', () => { @@ -607,9 +608,18 @@ describe('filter helpers', () => { expect(seriesPassesFilter(undefined, active, universe)).toBe(true); }); - it('treats an all-active dimension as unfiltered', () => { + it('checks explicit membership after the filter universe initializes', () => { const active = { engines: ['duckdb', 'datafusion'], formats: ['vortex', 'parquet'] }; expect(seriesPassesFilter({ engine: 'duckdb' }, active, universe)).toBe(true); + expect(seriesPassesFilter({ engine: 'new-engine' }, active, universe)).toBe(false); + expect(seriesPassesFilter({ format: 'new-format' }, active, universe)).toBe(false); + expect( + seriesPassesFilter( + { engine: 'duckdb' }, + { engines: [], formats: [] }, + { engines: [], formats: [] }, + ), + ).toBe(true); }); it('applies per-group overrides before the fallback visibility', () => { diff --git a/web/lib/chart-format.ts b/web/lib/chart-format.ts index c0c9bb0..c589874 100644 --- a/web/lib/chart-format.ts +++ b/web/lib/chart-format.ts @@ -956,12 +956,12 @@ export interface GlobalFilterState { /** * Parse one `?engine=` / `?format=` CSV param into a deduplicated, trimmed * allowlist, the TypeScript port of the Axum server's `parse_csv`. Empty - * entries (e.g. trailing commas) are dropped; an absent or entirely empty param - * means "no filter active" and is encoded as an empty array. + * entries (e.g. trailing commas) are dropped. An absent parameter returns null + * for the defaults; a present empty parameter returns an empty active set. */ -export function parseFilterCsv(raw: string | null | undefined): string[] { +export function parseFilterCsv(raw: string | null | undefined): string[] | null { if (raw === null || raw === undefined) { - return []; + return null; } const seen = new Set(); const out: string[] = []; @@ -977,16 +977,16 @@ export function parseFilterCsv(raw: string | null | undefined): string[] { } /** - * Translate a URL allowlist into the active chip set. An empty allowlist means - * "no filter", so every chip in the universe is active. A non-empty allowlist + * Translate a URL allowlist into the active chip set. A null allowlist uses + * the defaults, with every engine active. An explicit allowlist * is taken verbatim, even if a chip has since been added or removed from the * universe, which keeps stale URLs deterministic. */ export function seedActiveFromAllowlist( - allowlist: readonly string[], + allowlist: readonly string[] | null, universe: readonly string[], ): string[] { - return allowlist.length === 0 ? [...universe] : [...allowlist]; + return allowlist === null ? [...universe] : [...allowlist]; } /** @@ -1005,10 +1005,10 @@ export const DEFAULT_HIDDEN_FORMATS: readonly string[] = ['lance']; * [`seedActiveFromAllowlist`], which the engine dimension still uses directly. */ export function seedActiveFormats( - allowlist: readonly string[], + allowlist: readonly string[] | null, universe: readonly string[], ): string[] { - if (allowlist.length > 0) { + if (allowlist !== null) { return [...allowlist]; } return universe.filter((format) => !DEFAULT_HIDDEN_FORMATS.includes(format)); @@ -1016,9 +1016,9 @@ export function seedActiveFormats( /** * Whether a series passes the global filter. A series is hidden when its - * engine/format dimension is filtered (the active set is a strict subset of - * the universe) AND its tag is not in the active set. Series without an engine - * tag (e.g. compression-time `format:op` series) are unaffected by the engine + * engine/format dimension has initialized chips and its tag is not in the active + * set. Series without an engine tag (e.g. compression-time `format:op` series) + * are unaffected by the engine * filter, symmetric for format, so hiding an engine does not nuke charts that * have no engine dimension. */ @@ -1028,18 +1028,10 @@ export function seriesPassesFilter( universe: FilterUniverse, ): boolean { const m = meta ?? {}; - if ( - m.engine && - active.engines.length < universe.engines.length && - !active.engines.includes(m.engine) - ) { + if (m.engine && universe.engines.length > 0 && !active.engines.includes(m.engine)) { return false; } - if ( - m.format && - active.formats.length < universe.formats.length && - !active.formats.includes(m.format) - ) { + if (m.format && universe.formats.length > 0 && !active.formats.includes(m.format)) { return false; } return true; diff --git a/web/lib/chart-store.test.ts b/web/lib/chart-store.test.ts index b80516e..7a2382c 100644 --- a/web/lib/chart-store.test.ts +++ b/web/lib/chart-store.test.ts @@ -35,7 +35,7 @@ const UNIVERSE = { engines: ['datafusion', 'duckdb'], formats: ['parquet', 'vort describe('global filter store', () => { it('seeds every chip active with no URL allowlist', () => { - initGlobalFilter(UNIVERSE, [], []); + initGlobalFilter(UNIVERSE, null, null); const snap = getGlobalFilterSnapshot(); expect(snap.active.engines).toEqual(['datafusion', 'duckdb']); expect(snap.active.formats).toEqual(['parquet', 'vortex']); @@ -45,15 +45,15 @@ describe('global filter store', () => { const universe = { engines: ['datafusion', 'duckdb'], formats: ['parquet', 'vortex', 'lance'] }; // No `?format=` allowlist: lance is excluded by default (it is far slower, so // it buries the comparison); the rest of the format universe stays active. - initGlobalFilter(universe, [], []); + initGlobalFilter(universe, null, null); expect(getGlobalFilterSnapshot().active.formats).toEqual(['parquet', 'vortex']); // An explicit allowlist is taken verbatim, so a URL can pin lance back on. - initGlobalFilter(universe, [], ['lance']); + initGlobalFilter(universe, null, ['lance']); expect(getGlobalFilterSnapshot().active.formats).toEqual(['lance']); }); it('seeds verbatim from a URL allowlist and toggles chips independently', () => { - initGlobalFilter(UNIVERSE, ['duckdb'], []); + initGlobalFilter(UNIVERSE, ['duckdb'], null); expect(getGlobalFilterSnapshot().active.engines).toEqual(['duckdb']); toggleGlobalFilterValue('engine', 'datafusion'); @@ -72,7 +72,7 @@ describe('global filter store', () => { }); it('notifies subscribers with a fresh snapshot reference per mutation', () => { - initGlobalFilter(UNIVERSE, [], []); + initGlobalFilter(UNIVERSE, null, null); const before = getGlobalFilterSnapshot(); let notified = 0; const unsubscribe = subscribeGlobalFilter(() => { @@ -89,7 +89,7 @@ describe('global filter store', () => { describe('per-group store', () => { beforeEach(() => { - initGlobalFilter(UNIVERSE, [], []); + initGlobalFilter(UNIVERSE, null, null); }); it('restores a URL filter without discarding hydrated series metadata', () => { @@ -129,7 +129,7 @@ describe('per-group store', () => { it('restores a globally hidden series with a local visible override', () => { const slug = 'group-global-fallback'; const universe = { ...UNIVERSE, formats: [...UNIVERSE.formats, 'lance'] }; - initGlobalFilter(universe, [], []); + initGlobalFilter(universe, null, null); noteGroupSeries(slug, { lance: { format: 'lance' } }); expect(groupSeriesIsVisible(getGroupSnapshot(slug), 'lance')).toBe(false); @@ -164,7 +164,7 @@ describe('per-group store', () => { it('uses a macro to restore every match hidden by the global default', () => { const slug = 'group-global-macro'; const universe = { ...UNIVERSE, formats: [...UNIVERSE.formats, 'lance'] }; - initGlobalFilter(universe, [], []); + initGlobalFilter(universe, null, null); noteGroupSeries(slug, { 'datafusion:lance': { engine: 'datafusion', format: 'lance' }, 'duckdb:lance': { engine: 'duckdb', format: 'lance' }, diff --git a/web/lib/chart-store.ts b/web/lib/chart-store.ts index 4229a13..caea1fb 100644 --- a/web/lib/chart-store.ts +++ b/web/lib/chart-store.ts @@ -316,14 +316,14 @@ export function getGlobalFilterSnapshot(): GlobalFilterSnapshot { /** * Seed the store from server-provided props: the chip universe plus the URL - * `?engine=`/`?format=` allowlists (empty allowlist means every chip active). + * `?engine=`/`?format=` allowlists (null uses the defaults; empty hides all). * Called by the filter bar on mount and again on soft navigation, so the store * tracks the URL state of the page that most recently mounted it. */ export function initGlobalFilter( universe: FilterUniverse, - engineAllowlist: readonly string[], - formatAllowlist: readonly string[], + engineAllowlist: readonly string[] | null, + formatAllowlist: readonly string[] | null, ): void { globalSnapshot = { universe: { engines: [...universe.engines], formats: [...universe.formats] },