diff --git a/.changeset/userfilters-allowaddtab-session-tabs-os5236.md b/.changeset/userfilters-allowaddtab-session-tabs-os5236.md new file mode 100644 index 000000000..2de0eda4f --- /dev/null +++ b/.changeset/userfilters-allowaddtab-session-tabs-os5236.md @@ -0,0 +1,13 @@ +--- +'@object-ui/plugin-list': patch +--- + +`userFilters` tabs: the `allowAddTab` button now adds a tab instead of doing nothing (objectstack#5236) + +The affordance `allowAddTab` renders had hover styling and `title="Add filter tab"` but no `onClick`, and `TabFilters` took no add-tab callback at all — a control that looked fully clickable and did nothing, which disguises "not implemented" as "a bug where clicking does nothing". That mattered more once objectstack#5073 promoted `allowAddTab` into the spec's `UserFiltersSchema`: the key became discoverable through JSON Schema, the Studio SchemaForm and the reference docs, so an author writing `allowAddTab: true` gets a declaration the runtime did not honour. + +Clicking it now opens a small naming popover (the same Popover primitive the filter chips and the "More" overflow already use). Confirming a name adds a tab to the same bar as the presets, carrying a snapshot of the conditions applied at that moment, and selects it. Session tabs also carry a remove affordance; authored presets deliberately do not, since those are metadata. Removing the active session tab re-selects the author's default with the same precedence the initial mount uses, so the bar is never left with no active tab while the removed tab's conditions stay applied. + +The new tab is **session-scoped, held in component state** — no `sys_metadata` write, no API call, no web storage, per ADR-0047 ("an end user's filter choices are session-scoped and never become metadata"). `sessionStorage` was available and deliberately not used: `UserFilters` receives no object or view identity, so any storage key it could invent would be shared by every list in the browser tab, surfacing one list's ad-hoc tabs on another's bar. Persistence beyond the mount, if ever wanted, belongs to the host that already owns the session channel for filter selections (`onSelectionsChange` mirrored into `uf_*` URL params) and can key it by view. The synthetic tab id is reported through `onSelectionsChange` like any other tab switch, so a host mirroring it into the URL hands it back on the next mount, where the existing id check finds no such tab and falls back to the author's default. + +No public API change: `UserFiltersProps` is untouched, and `allowAddTab: false` / an omitted `allowAddTab` still render no affordance at all. diff --git a/packages/plugin-list/src/UserFilters.tsx b/packages/plugin-list/src/UserFilters.tsx index 19e220c04..bf49a995b 100644 --- a/packages/plugin-list/src/UserFilters.tsx +++ b/packages/plugin-list/src/UserFilters.tsx @@ -7,7 +7,7 @@ */ import * as React from 'react'; -import { cn, Button, Popover, PopoverContent, PopoverTrigger, LookupValuePicker } from '@object-ui/components'; +import { cn, Button, Input, Popover, PopoverContent, PopoverTrigger, LookupValuePicker } from '@object-ui/components'; import { ChevronDown, X, Plus } from 'lucide-react'; import type { ListViewSchema } from '@object-ui/types'; import { normalizeFilterOperator } from '@objectstack/spec/ui'; @@ -708,6 +708,16 @@ interface TabFiltersProps { onSelectionsChange?: (selections: Record>) => void; } +/** + * A tab the END USER added at runtime through `allowAddTab` — never an + * authored preset, never metadata. See {@link TabFilters} for the scope rules. + */ +interface SessionTab { + id: string; + label: string; + filters: any[]; +} + function TabFilters({ tabs, showAllRecords, allowAddTab, onFilterChange, className, initialTab, onSelectionsChange }: TabFiltersProps) { const [activeTab, setActiveTab] = React.useState(() => { // URL-restored tab wins over the author's default. @@ -718,18 +728,49 @@ function TabFilters({ tabs, showAllRecords, allowAddTab, onFilterChange, classNa return defaultTab?.id || (showAllRecords ? '__all__' : tabs[0]?.id || ''); }); + /** + * User-added tabs (`allowAddTab`), held in **component state only**. + * + * ADR-0047 scopes an end user's filter choices to the session and forbids + * them ever becoming metadata, so this renderer stays a metadata READER: + * adding a tab writes no `sys_metadata`, calls no API, and touches no + * storage. Component state (not `sessionStorage`) is the deliberate pick — + * `UserFilters` receives no object/view identity, so a shared + * `sessionStorage` key would surface one list's ad-hoc tabs on another + * list's bar in the same browser tab. Persistence beyond the mount, if it + * is ever wanted, belongs to the host that already owns the session channel + * for filter selections (`onSelectionsChange` → `uf_*` URL params) and can + * key it by view. + */ + const [sessionTabs, setSessionTabs] = React.useState([]); + const [addOpen, setAddOpen] = React.useState(false); + const [draftLabel, setDraftLabel] = React.useState(''); + + /** + * Conditions currently applied for `tabId` — the preset's filters, the + * session tab's snapshot, or `[]` for the synthetic "All records" tab (and + * for an id nothing answers to, which is how a stale restored `_tab` + * degrades). In tabs mode this IS the whole filter state of the bar: the + * component owns no other filter surface. + */ + const filtersForTab = React.useCallback( + (tabId: string): any[] => { + if (tabId === '__all__') return []; + // `normalizeTabPresets` guarantees `id` on every preset it emits. + const preset = tabs.find(t => t.id === tabId); + if (preset) return preset.filters || []; + return sessionTabs.find(t => t.id === tabId)?.filters || []; + }, + [tabs, sessionTabs], + ); + const handleTabChange = React.useCallback( (tabId: string) => { setActiveTab(tabId); - if (tabId === '__all__') { - onFilterChange([]); - } else { - const tab = tabs.find(t => t.id === tabId); - onFilterChange(tab?.filters || []); - } + onFilterChange(filtersForTab(tabId)); onSelectionsChange?.({ _tab: [tabId] }); }, - [tabs, onFilterChange, onSelectionsChange], + [filtersForTab, onFilterChange, onSelectionsChange], ); const allTabs = React.useMemo(() => { @@ -740,6 +781,60 @@ function TabFilters({ tabs, showAllRecords, allowAddTab, onFilterChange, classNa return result; }, [tabs, showAllRecords]); + /** + * Confirm the naming input: snapshot the conditions currently applied under + * the typed label and select the new tab. + * + * The snapshot source is the active tab's conditions because that is the + * entire filter state tabs mode carries (see {@link filtersForTab}) — the + * new tab therefore reproduces exactly the rows the user is looking at when + * they press Add. Each condition is copied so a later preset change cannot + * alias into the session tab. + * + * The synthetic id is reported through `onSelectionsChange` like any other + * tab switch, so the host's mirror stays truthful. A host that persists it + * (`uf__tab` in the URL) hands it back as `initialTab` on the next mount, + * where the existing id check finds no such tab and falls back to the + * author's default — a session tab cannot outlive the mount by the back door. + */ + const handleAddTab = React.useCallback(() => { + const label = draftLabel.trim(); + if (!label) return; + // Synthetic, session-only id. `__…__` mirrors the "__all__" spelling this + // component already uses for the tab it invents; the loop keeps it clear + // of author-defined preset ids. + const taken = new Set([ + ...tabs.map(t => t.id ?? t.name ?? ''), + ...sessionTabs.map(t => t.id), + ]); + let seq = sessionTabs.length + 1; + while (taken.has(`__session_${seq}__`)) seq += 1; + const id = `__session_${seq}__`; + const snapshot = filtersForTab(activeTab).map(c => (Array.isArray(c) ? [...c] : c)); + setSessionTabs(prev => [...prev, { id, label, filters: snapshot }]); + setActiveTab(id); + setDraftLabel(''); + setAddOpen(false); + onFilterChange(snapshot); + onSelectionsChange?.({ _tab: [id] }); + }, [draftLabel, tabs, sessionTabs, filtersForTab, activeTab, onFilterChange, onSelectionsChange]); + + /** + * Drop a session tab. Removing the ACTIVE one re-selects the author's + * default with the same precedence the initial mount uses, so the bar is + * never left with no active tab while the removed tab's conditions stay + * applied. Presets have no remove affordance — they are metadata. + */ + const handleRemoveTab = React.useCallback( + (tabId: string) => { + setSessionTabs(prev => prev.filter(t => t.id !== tabId)); + if (activeTab !== tabId) return; + const defaultTab = tabs.find(t => t.default); + handleTabChange(defaultTab?.id || (showAllRecords ? '__all__' : tabs[0]?.id || '')); + }, + [activeTab, tabs, showAllRecords, handleTabChange], + ); + // Emit the initially-active tab's filters on mount (restored tab or // author default — `activeTab` already resolved the precedence). React.useEffect(() => { @@ -772,14 +867,91 @@ function TabFilters({ tabs, showAllRecords, allowAddTab, onFilterChange, classNa ); })} + {/* User-added tabs, in the same bar as the presets. They carry a remove + affordance (presets don't — those are metadata), so the pill is a + wrapper around two buttons rather than one button. */} + {sessionTabs.map(tab => { + const isActive = activeTab === tab.id; + return ( + + + + + ); + })} {allowAddTab && ( - + + + + +

+ Name this tab. It keeps the filters applied right now, and lives in this session only. +

+ setDraftLabel(e.target.value)} + onKeyDown={e => { + if (e.key === 'Enter') { + e.preventDefault(); + handleAddTab(); + } + }} + placeholder="Tab name" + className="h-7 text-xs" + data-testid="filter-tab-add-input" + /> +
+ +
+ + )} ); diff --git a/packages/plugin-list/src/__tests__/UserFilters.addTab.test.tsx b/packages/plugin-list/src/__tests__/UserFilters.addTab.test.tsx new file mode 100644 index 000000000..9c9e2b011 --- /dev/null +++ b/packages/plugin-list/src/__tests__/UserFilters.addTab.test.tsx @@ -0,0 +1,314 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectstack#5236 — `allowAddTab` rendered a dead button. + * + * The affordance had hover styling and `title="Add filter tab"` but no + * `onClick`, and the component took no add-tab callback: a control that looked + * fully clickable and did nothing, which disguises "not implemented" as "a bug + * where clicking does nothing". Maintainer ruling **A1** (2026-08-06) wired it + * up instead of removing it: naming input, snapshot of the filters currently + * applied, and the new tab is **session-scoped**. + * + * Session scope here means **component state** — no metadata, no API, no + * storage (ADR-0047: an end user's filter choices are session-scoped and never + * become metadata). `sessionStorage` was available and deliberately not used: + * `UserFilters` receives no object/view identity, so any storage key it could + * invent would be shared by every list in the browser tab. These tests pin + * that decision from both sides — nothing is written (`Storage.setItem`, the + * data source, `fetch` all stay at zero calls) and nothing survives a remount. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup } from '@testing-library/react'; +import { SchemaRendererProvider } from '@object-ui/react'; +import { UserFilters } from '../UserFilters'; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +/** Two authored presets, the first one the author's default. */ +const tabsConfig = { + element: 'tabs' as const, + tabs: [ + { + name: 'open', + label: 'Open', + isDefault: true, + filter: [{ field: 'status', operator: 'equals', value: 'todo' }], + }, + { + name: 'urgent', + label: 'Urgent', + filter: [{ field: 'priority', operator: 'equals', value: 'urgent' }], + }, + ], +}; + +const OPEN_CONDITIONS = [['status', 'equals', 'todo']]; +const URGENT_CONDITIONS = [['priority', 'equals', 'urgent']]; + +/** Open the naming popover, type `label`, confirm. */ +function addTab(label: string) { + fireEvent.click(screen.getByTestId('filter-tab-add')); + fireEvent.change(screen.getByTestId('filter-tab-add-input'), { target: { value: label } }); + fireEvent.click(screen.getByTestId('filter-tab-add-confirm')); +} + +/** The pill wrapper carries the active styling for a session tab. */ +function pillClassName(tabId: string): string { + return screen.getByTestId(`filter-tab-${tabId}`).parentElement?.className ?? ''; +} + +describe('UserFilters tabs — allowAddTab (objectstack#5236, ruling A1)', () => { + it('clicking add opens a naming input; confirming snapshots the applied filters into a new, selected tab', () => { + const onFilterChange = vi.fn(); + const onSelectionsChange = vi.fn(); + render( + , + ); + + // Move off the default so the snapshot has something distinguishable in it. + fireEvent.click(screen.getByTestId('filter-tab-urgent')); + expect(onFilterChange).toHaveBeenLastCalledWith(URGENT_CONDITIONS); + + // The add affordance now opens a naming input rather than doing nothing. + fireEvent.click(screen.getByTestId('filter-tab-add')); + expect(screen.getByTestId('filter-tab-add-content')).toBeTruthy(); + const input = screen.getByTestId('filter-tab-add-input') as HTMLInputElement; + // Empty name cannot be confirmed. + expect((screen.getByTestId('filter-tab-add-confirm') as HTMLButtonElement).disabled).toBe(true); + + fireEvent.change(input, { target: { value: ' My urgent ' } }); + expect((screen.getByTestId('filter-tab-add-confirm') as HTMLButtonElement).disabled).toBe(false); + fireEvent.click(screen.getByTestId('filter-tab-add-confirm')); + + // The new tab renders in the same bar, label trimmed, and is selected. + const added = screen.getByTestId('filter-tab-__session_1__'); + expect(added.textContent).toBe('My urgent'); + expect(added.closest('[data-testid="user-filters-tabs"]')).toBeTruthy(); + expect(pillClassName('__session_1__')).toContain('bg-primary'); + expect(onSelectionsChange).toHaveBeenLastCalledWith({ _tab: ['__session_1__'] }); + + // …carrying the conditions that were applied when it was created. + expect(onFilterChange).toHaveBeenLastCalledWith(URGENT_CONDITIONS); + }); + + it('the snapshot is stored, not recomputed — switching away and back re-emits the captured conditions', () => { + const onFilterChange = vi.fn(); + render( + , + ); + + fireEvent.click(screen.getByTestId('filter-tab-urgent')); + addTab('Snapshot'); + + fireEvent.click(screen.getByTestId('filter-tab-open')); + expect(onFilterChange).toHaveBeenLastCalledWith(OPEN_CONDITIONS); + + fireEvent.click(screen.getByTestId('filter-tab-__session_1__')); + expect(onFilterChange).toHaveBeenLastCalledWith(URGENT_CONDITIONS); + expect(pillClassName('__session_1__')).toContain('bg-primary'); + }); + + it('snapshots an empty condition set when "All records" is the active tab', () => { + const onFilterChange = vi.fn(); + render( + , + ); + + fireEvent.click(screen.getByTestId('filter-tab-__all__')); + addTab('Everything'); + expect(onFilterChange).toHaveBeenLastCalledWith([]); + + fireEvent.click(screen.getByTestId('filter-tab-urgent')); + fireEvent.click(screen.getByTestId('filter-tab-__session_1__')); + expect(onFilterChange).toHaveBeenLastCalledWith([]); + }); + + it('confirms on Enter as well as on the button', () => { + render( + , + ); + + fireEvent.click(screen.getByTestId('filter-tab-add')); + fireEvent.change(screen.getByTestId('filter-tab-add-input'), { target: { value: 'Typed' } }); + fireEvent.keyDown(screen.getByTestId('filter-tab-add-input'), { key: 'Enter' }); + + expect(screen.getByTestId('filter-tab-__session_1__').textContent).toBe('Typed'); + }); + + it('adds a second tab under its own id', () => { + render( + , + ); + + addTab('First'); + addTab('Second'); + + expect(screen.getByTestId('filter-tab-__session_1__').textContent).toBe('First'); + expect(screen.getByTestId('filter-tab-__session_2__').textContent).toBe('Second'); + }); +}); + +describe('UserFilters tabs — adding a tab writes nothing (ADR-0047)', () => { + it('calls no data-source method, no fetch, and no web storage', () => { + const dataSource = { + find: vi.fn().mockResolvedValue([]), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + }; + const fetchSpy = vi.fn(); + vi.stubGlobal('fetch', fetchSpy); + // Storage.prototype covers both sessionStorage and localStorage. + const setItem = vi.spyOn(Storage.prototype, 'setItem'); + + render( + + + , + ); + + fireEvent.click(screen.getByTestId('filter-tab-urgent')); + addTab('No metadata please'); + expect(screen.getByTestId('filter-tab-__session_1__')).toBeTruthy(); + + // A session tab is not metadata: no write API of any shape was reached. + expect(dataSource.create).toHaveBeenCalledTimes(0); + expect(dataSource.update).toHaveBeenCalledTimes(0); + expect(dataSource.delete).toHaveBeenCalledTimes(0); + expect(dataSource.find).toHaveBeenCalledTimes(0); + expect(dataSource.findOne).toHaveBeenCalledTimes(0); + expect(fetchSpy).toHaveBeenCalledTimes(0); + expect(setItem).toHaveBeenCalledTimes(0); + }); +}); + +describe('UserFilters tabs — session tabs do not survive a remount', () => { + it('drops the added tab and falls back to the author default, even when the host mirrored the tab id back', () => { + const first = vi.fn(); + const { unmount } = render( + , + ); + fireEvent.click(screen.getByTestId('filter-tab-urgent')); + addTab('Ephemeral'); + expect(screen.getByTestId('filter-tab-__session_1__')).toBeTruthy(); + + unmount(); + + // The host persists `_tab` (uf_* URL params), so it hands the synthetic id + // straight back. The tab itself is gone and the restored id resolves to + // nothing, so the author's default wins — no empty bar, no stale filters. + const second = vi.fn(); + render( + , + ); + + expect(screen.queryByTestId('filter-tab-__session_1__')).toBeNull(); + expect(screen.queryByText('Ephemeral')).toBeNull(); + expect(second).toHaveBeenLastCalledWith(OPEN_CONDITIONS); + expect(screen.getByTestId('filter-tab-open').className).toContain('bg-primary'); + }); +}); + +describe('UserFilters tabs — session tabs can be removed', () => { + it('removing the active session tab re-selects the author default and re-emits its filters', () => { + const onFilterChange = vi.fn(); + render( + , + ); + + fireEvent.click(screen.getByTestId('filter-tab-urgent')); + addTab('Temporary'); + fireEvent.click(screen.getByTestId('filter-tab-remove-__session_1__')); + + expect(screen.queryByTestId('filter-tab-__session_1__')).toBeNull(); + expect(onFilterChange).toHaveBeenLastCalledWith(OPEN_CONDITIONS); + expect(screen.getByTestId('filter-tab-open').className).toContain('bg-primary'); + }); + + it('removing an inactive session tab leaves the applied filters alone', () => { + const onFilterChange = vi.fn(); + render( + , + ); + + addTab('Temporary'); + fireEvent.click(screen.getByTestId('filter-tab-urgent')); + const callsBefore = onFilterChange.mock.calls.length; + + fireEvent.click(screen.getByTestId('filter-tab-remove-__session_1__')); + + expect(screen.queryByTestId('filter-tab-__session_1__')).toBeNull(); + expect(onFilterChange.mock.calls.length).toBe(callsBefore); + expect(screen.getByTestId('filter-tab-urgent').className).toContain('bg-primary'); + }); + + it('offers no remove affordance on authored presets', () => { + render( + , + ); + + expect(screen.queryByTestId('filter-tab-remove-open')).toBeNull(); + expect(screen.queryByTestId('filter-tab-remove-urgent')).toBeNull(); + expect(screen.queryByTestId('filter-tab-remove-__all__')).toBeNull(); + }); +}); + +describe('UserFilters tabs — allowAddTab off (regression)', () => { + it('renders no add affordance when allowAddTab is false', () => { + render( + , + ); + expect(screen.getByTestId('user-filters-tabs')).toBeTruthy(); + expect(screen.queryByTestId('filter-tab-add')).toBeNull(); + }); + + it('renders no add affordance when allowAddTab is omitted', () => { + render(); + expect(screen.getByTestId('user-filters-tabs')).toBeTruthy(); + expect(screen.queryByTestId('filter-tab-add')).toBeNull(); + }); +});