From e40bf39b9c53fd30b984c781051c0646b36304a8 Mon Sep 17 00:00:00 2001 From: Ali Waseem Date: Thu, 5 Mar 2026 10:42:17 -0700 Subject: [PATCH] fix: updated filter bar logic to support selection of columns once set (#43408) ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? - Allow selecting the column, it respects different types and updates the values correctly. If the values are different it gets wiped due to mismatches - Updated E2E tests and unit tests --- e2e/studio/features/filter-bar.spec.ts | 143 ++++++++++++- e2e/studio/utils/filter-bar-helpers.ts | 18 ++ .../src/FilterBar/FilterBar.test.tsx | 108 ++++++++-- .../src/FilterBar/FilterBarContext.tsx | 53 +++-- .../src/FilterBar/FilterCondition.tsx | 110 +++++++++- .../ui-patterns/src/FilterBar/FilterGroup.tsx | 7 +- .../ui-patterns/src/FilterBar/menuItems.ts | 13 ++ packages/ui-patterns/src/FilterBar/types.ts | 7 + .../src/FilterBar/useKeyboardNavigation.ts | 11 +- .../ui-patterns/src/FilterBar/utils.test.ts | 195 ++++++++++++++++++ packages/ui-patterns/src/FilterBar/utils.ts | 49 +++++ 11 files changed, 665 insertions(+), 49 deletions(-) diff --git a/e2e/studio/features/filter-bar.spec.ts b/e2e/studio/features/filter-bar.spec.ts index 5c186bf9f2a..f9f52076cbd 100644 --- a/e2e/studio/features/filter-bar.spec.ts +++ b/e2e/studio/features/filter-bar.spec.ts @@ -10,6 +10,7 @@ import { selectOperator, selectOperatorByClick, setupFilterBarPage, + switchProperty, } from '../utils/filter-bar-helpers.js' import { test } from '../utils/test.js' import { toUrl } from '../utils/to-url.js' @@ -166,7 +167,6 @@ test.describe('Filter Bar', () => { await freeformInput.click() await expect(freeformInput).toBeFocused() - // Tab should exit the filter bar (freeform input should lose focus) await page.keyboard.press('Tab') await expect(freeformInput).not.toBeFocused() @@ -511,14 +511,12 @@ test.describe('Filter Bar', () => { const valueInput = page.getByTestId(`filter-value-${columnName}`) await valueInput.fill('HelloWorld') - // Move cursor to middle (after "Hello") and insert text await page.keyboard.press('Home') for (let i = 0; i < 5; i++) { await page.keyboard.press('ArrowRight') } await page.keyboard.type('_Middle_') - // Verify the text was inserted in the middle, not appended at the end await expect(valueInput).toHaveValue('Hello_Middle_World') } finally { await dropTable(tableName) @@ -777,7 +775,6 @@ test.describe('Filter Bar', () => { await selectColumnFilter(page, 'is_active') await selectOperator(page, 'is_active', '=') - // Navigate to 'false' option (second item) and select with Enter await page.keyboard.press('ArrowDown') await page.keyboard.press('Enter') @@ -803,7 +800,6 @@ test.describe('Filter Bar', () => { await freeformInput.click() await expect(freeformInput).toBeFocused() - // Shift+Tab should exit the filter bar (freeform input should lose focus) await page.keyboard.press('Shift+Tab') await expect(freeformInput).not.toBeFocused() @@ -925,10 +921,8 @@ test.describe('Filter Bar', () => { const removeButton = page.getByRole('button', { name: 'Remove filters' }) await expect(removeButton).toBeVisible() - // Clicking "Remove filters" should clear filters and restore data await removeButton.click() - // Filter pill should be gone and data should be visible again await expect(page.getByTestId('filter-condition-name')).not.toBeVisible({ timeout: 10000 }) await expect(page.getByRole('gridcell', { name: 'Alice' })).toBeVisible({ timeout: 10000 }) await expect(page.getByRole('gridcell', { name: 'Bob' })).toBeVisible() @@ -937,4 +931,139 @@ test.describe('Filter Bar', () => { } }) }) + + test.describe('Property Switching', () => { + test('clicking property label opens picker and selecting new column switches the filter', async ({ + page, + ref, + }) => { + const tableName = `${tableNamePrefix}_prop_switch` + + await query( + `CREATE TABLE IF NOT EXISTS ${tableName} ( + id bigint generated by default as identity primary key, + first_name text, + last_name text + )` + ) + await query( + `INSERT INTO ${tableName} (first_name, last_name) VALUES ('Alice', 'Smith'), ('Bob', 'Jones')` + ) + + try { + await setupFilterBarPage(page, ref, toUrl(`/project/${ref}/editor?schema=public`)) + await navigateToTable(page, ref, tableName) + + await addFilter(page, ref, 'first_name', '=', 'Alice') + await expect(page.getByTestId('filter-condition-first_name')).toBeVisible() + + await switchProperty(page, 'first_name', 'last_name') + + await expect(page.getByTestId('filter-condition-first_name')).not.toBeVisible() + await expect(page.getByTestId('filter-condition-last_name')).toBeVisible() + } finally { + await dropTable(tableName) + } + }) + + test('switching between same-type columns preserves compatible operator and value', async ({ + page, + ref, + }) => { + const tableName = `${tableNamePrefix}_prop_compat` + + await query( + `CREATE TABLE IF NOT EXISTS ${tableName} ( + id bigint generated by default as identity primary key, + first_name text, + last_name text + )` + ) + await query( + `INSERT INTO ${tableName} (first_name, last_name) VALUES ('Alice', 'Smith'), ('Bob', 'Smith'), ('Alice', 'Jones')` + ) + + try { + await setupFilterBarPage(page, ref, toUrl(`/project/${ref}/editor?schema=public`)) + await navigateToTable(page, ref, tableName) + + await addFilter(page, ref, 'first_name', '=', 'Alice') + await expect(page.getByRole('gridcell', { name: 'Bob' })).not.toBeVisible() + + // '=' is compatible with text→text, so operator and value ("Alice") are preserved + const rowsWaiter = createApiResponseWaiter(page, 'pg-meta', ref, 'query?key=table-rows-') + await switchProperty(page, 'first_name', 'last_name') + await rowsWaiter + + // last_name = Alice matches no rows (last names are Smith/Jones) + const rows = page.locator('[role="row"]') + await expect(rows).toHaveCount(1) // header only + } finally { + await dropTable(tableName) + } + }) + + test('switching to column with incompatible operator resets operator', async ({ + page, + ref, + }) => { + const tableName = `${tableNamePrefix}_prop_incompat` + + await query( + `CREATE TABLE IF NOT EXISTS ${tableName} ( + id bigint generated by default as identity primary key, + name text, + is_active boolean + )` + ) + await query( + `INSERT INTO ${tableName} (name, is_active) VALUES ('Alice', true), ('Bob', false)` + ) + + try { + await setupFilterBarPage(page, ref, toUrl(`/project/${ref}/editor?schema=public`)) + await navigateToTable(page, ref, tableName) + + // '~~' (Like) only exists on text columns, not boolean + await selectColumnFilter(page, 'name') + await selectOperatorByClick(page, 'name', '~~') + const valueInput = page.getByTestId('filter-value-name') + await valueInput.fill('Alice') + await page.keyboard.press('Enter') + + await switchProperty(page, 'name', 'is_active') + + await expect(page.getByTestId('filter-operator-is_active')).toBeFocused() + } finally { + await dropTable(tableName) + } + }) + + test('Escape closes property picker without changing the filter', async ({ page, ref }) => { + const tableName = `${tableNamePrefix}_prop_esc` + const columnName = 'name' + + await createTable(tableName, columnName, [{ name: 'Alice' }, { name: 'Bob' }]) + + try { + await setupFilterBarPage(page, ref, toUrl(`/project/${ref}/editor?schema=public`)) + await navigateToTable(page, ref, tableName) + + await addFilter(page, ref, columnName, '=', 'Alice') + + const conditionEl = page.getByTestId(`filter-condition-${columnName}`) + await conditionEl.locator('span', { hasText: columnName }).first().click() + + const searchInput = page.getByTestId(`filter-property-search-${columnName}`) + await expect(searchInput).toBeVisible() + + await page.keyboard.press('Escape') + + await expect(page.getByTestId(`filter-condition-${columnName}`)).toBeVisible() + await expect(searchInput).not.toBeVisible() + } finally { + await dropTable(tableName) + } + }) + }) }) diff --git a/e2e/studio/utils/filter-bar-helpers.ts b/e2e/studio/utils/filter-bar-helpers.ts index 8a9edbbc190..140e5e00295 100644 --- a/e2e/studio/utils/filter-bar-helpers.ts +++ b/e2e/studio/utils/filter-bar-helpers.ts @@ -73,6 +73,24 @@ export async function addFilterWithDropdownValue( await rowsWaiter } +export async function switchProperty(page: Page, currentColumnName: string, newColumnName: string) { + const conditionEl = page.getByTestId(`filter-condition-${currentColumnName}`) + await conditionEl + .locator(`span`, { hasText: new RegExp(`^${currentColumnName}$`, 'i') }) + .first() + .click() + + const searchInput = page.getByTestId(`filter-property-search-${currentColumnName}`) + await expect(searchInput).toBeVisible() + + await searchInput.fill(newColumnName) + + await expect(page.getByTestId(`filter-menu-item-${newColumnName}`)).toBeVisible() + await page.getByTestId(`filter-menu-item-${newColumnName}`).click() + + await expect(page.getByTestId(`filter-condition-${newColumnName}`)).toBeVisible() +} + export async function setupFilterBarPage(page: Page, ref: string, editorUrl: string) { const loadPromise = waitForTableToLoad(page, ref) await page.goto(editorUrl) diff --git a/packages/ui-patterns/src/FilterBar/FilterBar.test.tsx b/packages/ui-patterns/src/FilterBar/FilterBar.test.tsx index 5504c209f52..a84b0afb613 100644 --- a/packages/ui-patterns/src/FilterBar/FilterBar.test.tsx +++ b/packages/ui-patterns/src/FilterBar/FilterBar.test.tsx @@ -90,19 +90,15 @@ describe('FilterBar', () => { freeform.focus() await user.click(freeform) - // Should show property items in popover expect(await screen.findByText('Name')).toBeInTheDocument() expect(screen.getByText('Status')).toBeInTheDocument() - // Select a property await user.click(screen.getByText('Status')) - // Wait for filter change callback and re-render with new state await waitFor(() => { expect(handleFilterChange).toHaveBeenCalled() }) - // Re-render with updated filters rerender( { /> ) - // Value input should appear for selected property await waitFor(() => { expect(screen.getByLabelText('Value for Status')).toBeInTheDocument() }) @@ -159,13 +154,10 @@ describe('FilterBar', () => { }) valueInput.focus() - // Popover should show value options expect(await screen.findByText('active')).toBeInTheDocument() - // Select 'active' (first item) await user.keyboard('{Enter}') - // Wait for value to be updated await waitFor(() => { expect(handleFilterChange).toHaveBeenCalledTimes(2) // Once for property, once for value }) @@ -244,19 +236,16 @@ describe('FilterBar', () => { /> ) - // Wait for FilterCondition to be created const valueInput = await waitFor(() => screen.getByLabelText('Value for Tag'), { timeout: 3000, }) - // Focus the value input to show the popover await user.click(valueInput) - // Custom UI should render inside the popover automatically (no menu for single custom option) + // When value options contain only a custom component, the popover renders it directly (no menu) const pickFoo = await screen.findByText('Pick Foo') await user.click(pickFoo) - // Wait for value change callback await waitFor(() => { expect(handleFilterChange).toHaveBeenCalledTimes(2) // Once for property, once for value }) @@ -271,7 +260,6 @@ describe('FilterBar', () => { /> ) - // Value should be applied const updatedValueInput = await screen.findByLabelText('Value for Tag') expect((updatedValueInput as HTMLInputElement).value).toBe('foo') }) @@ -361,6 +349,99 @@ describe('FilterBar', () => { expect(screen.queryByText('AND')).not.toBeInTheDocument() }) + it('allows switching filter property by clicking the label', async () => { + const user = userEvent.setup() + let currentFilters: FilterGroup = { + logicalOperator: 'AND', + conditions: [ + { + propertyName: 'name', + value: 'test', + operator: '=', + }, + ], + } + const handleFilterChange = vi.fn((filters) => { + currentFilters = filters + }) + + const { rerender } = render( + + ) + + expect(screen.getByText('Name')).toBeInTheDocument() + expect(screen.getByDisplayValue('test')).toBeInTheDocument() + + await user.click(screen.getByText('Name')) + + // Dropdown excludes current property ("Name" is visible as label but not in the picker list) + expect(await screen.findByText('Status')).toBeInTheDocument() + expect(screen.getByText('Count')).toBeInTheDocument() + + await user.click(screen.getByText('Status')) + + await waitFor(() => { + expect(handleFilterChange).toHaveBeenCalled() + }) + + const updatedCondition = currentFilters.conditions[0] as { + propertyName: string + value: string + operator: string + } + expect(updatedCondition.propertyName).toBe('status') + expect(updatedCondition.operator).toBe('=') + }) + + it('resets operator when switching to property with incompatible operators', async () => { + const user = userEvent.setup() + // CONTAINS exists on Name but not Status, so switching should reset operator + let currentFilters: FilterGroup = { + logicalOperator: 'AND', + conditions: [ + { + propertyName: 'name', + value: 'test', + operator: 'CONTAINS', + }, + ], + } + const handleFilterChange = vi.fn((filters) => { + currentFilters = filters + }) + + render( + + ) + + await user.click(screen.getByText('Name')) + await user.click(await screen.findByText('Status')) + + await waitFor(() => { + expect(handleFilterChange).toHaveBeenCalled() + }) + + const updatedCondition = currentFilters.conditions[0] as { + propertyName: string + value: string + operator: string + } + expect(updatedCondition.propertyName).toBe('status') + expect(updatedCondition.operator).toBe('') + }) + it('hides logical operators by default', () => { const multipleFilters: FilterGroup = { logicalOperator: 'AND', @@ -385,7 +466,6 @@ describe('FilterBar', () => { onFilterChange={mockOnFilterChange} freeformText="" onFreeformTextChange={mockOnFreeformTextChange} - // supportsOperators defaults to false /> ) diff --git a/packages/ui-patterns/src/FilterBar/FilterBarContext.tsx b/packages/ui-patterns/src/FilterBar/FilterBarContext.tsx index a9333cb31df..f9212a9132c 100644 --- a/packages/ui-patterns/src/FilterBar/FilterBarContext.tsx +++ b/packages/ui-patterns/src/FilterBar/FilterBarContext.tsx @@ -17,13 +17,14 @@ import { findConditionByPath, isAsyncOptionsFunction, removeFromGroup, + resolvePropertyChange, updateNestedLogicalOperator, updateNestedOperator, + updateNestedPropertyName, updateNestedValue, } from './utils' export type FilterBarContextValue = { - // Core state filters: FilterGroup filterProperties: FilterProperty[] activeInput: ActiveInputState @@ -32,7 +33,6 @@ export type FilterBarContextValue = { error: string | null highlightedConditionPath: number[] | null - // Handlers onFilterChange: (filters: FilterGroup) => void onFreeformTextChange: (text: string) => void setActiveInput: (input: ActiveInputState) => void @@ -47,9 +47,9 @@ export type FilterBarContextValue = { handleInputBlur: () => void handleGroupFreeformChange: (path: number[], value: string) => void handleLabelClick: (path: number[]) => void + handlePropertyChange: (path: number[], newPropertyName: string) => void handleLogicalOperatorChange: (path: number[]) => void - // Options cache propertyOptionsCache: Record< string, { options: (string | FilterOptionObject)[]; searchValue: string } @@ -58,13 +58,11 @@ export type FilterBarContextValue = { loadPropertyOptions: (property: FilterProperty, search: string) => void optionsError: string | null - // Config supportsOperators: boolean variant: FilterBarVariant actions?: FilterBarAction[] icon?: React.ReactNode - // Refs rootRef: React.RefObject } @@ -240,14 +238,12 @@ export function FilterBarRoot({ } setIsCommandMenuVisible(false) setActiveInput(null) - // Clear highlight when clicking outside setHighlightedConditionPath(null) }, 0) }, [setIsCommandMenuVisible, setActiveInput, hideTimeoutRef, setHighlightedConditionPath]) const handleGroupFreeformChange = useCallback( (_path: number[], value: string) => { - // Clear highlight when user types if (highlightedConditionPath) { setHighlightedConditionPath(null) } @@ -258,9 +254,40 @@ export function FilterBarRoot({ const handleLabelClick = useCallback( (path: number[]) => { - setActiveInput({ type: 'value', path }) + setActiveInput({ type: 'property', path }) + setIsCommandMenuVisible(true) + if (hideTimeoutRef.current) { + clearTimeout(hideTimeoutRef.current) + } }, - [setActiveInput] + [setActiveInput, setIsCommandMenuVisible, hideTimeoutRef] + ) + + const handlePropertyChange = useCallback( + (path: number[], newPropertyName: string) => { + const condition = findConditionByPath(filters, path) + if (!condition) return + + const newProperty = filterProperties.find((p) => p.name === newPropertyName) + if (!newProperty) return + + const { operator, value, focusTarget } = resolvePropertyChange( + condition.operator, + (condition.value ?? '').toString(), + newProperty + ) + + const updatedFilters = updateNestedPropertyName( + filters, + path, + newPropertyName, + operator, + value + ) + onFilterChange(updatedFilters) + setActiveInput({ type: focusTarget, path }) + }, + [filters, filterProperties, onFilterChange, setActiveInput] ) const handleLogicalOperatorChange = useCallback( @@ -292,7 +319,6 @@ export function FilterBarRoot({ const loading = externalLoading ?? isLoading const contextValue: FilterBarContextValue = { - // Core state filters, filterProperties, activeInput, @@ -301,7 +327,6 @@ export function FilterBarRoot({ error, highlightedConditionPath, - // Handlers onFilterChange, onFreeformTextChange, setActiveInput, @@ -316,27 +341,25 @@ export function FilterBarRoot({ handleInputBlur, handleGroupFreeformChange, handleLabelClick, + handlePropertyChange, handleLogicalOperatorChange, - // Options cache propertyOptionsCache, loadingOptions, loadPropertyOptions, optionsError, - // Config supportsOperators, variant, actions, icon, - // Refs rootRef, } return ( -
+
{children}
diff --git a/packages/ui-patterns/src/FilterBar/FilterCondition.tsx b/packages/ui-patterns/src/FilterBar/FilterCondition.tsx index 4a7d4c7cdeb..32b279e143c 100644 --- a/packages/ui-patterns/src/FilterBar/FilterCondition.tsx +++ b/packages/ui-patterns/src/FilterBar/FilterCondition.tsx @@ -14,7 +14,7 @@ import { import { DefaultCommandList } from './DefaultCommandList' import { useFilterBar } from './FilterBarContext' import { useDeferredBlur, useHighlightNavigation } from './hooks' -import { buildOperatorItems, buildValueItems } from './menuItems' +import { buildOperatorItems, buildPropertyChangeItems, buildValueItems } from './menuItems' import { FilterCondition as FilterConditionType } from './types' export type FilterConditionProps = { @@ -22,6 +22,7 @@ export type FilterConditionProps = { path: number[] isActive: boolean isOperatorActive: boolean + isPropertyActive: boolean isHighlighted: boolean } @@ -30,6 +31,7 @@ export function FilterCondition({ path, isActive, isOperatorActive, + isPropertyActive, isHighlighted, }: FilterConditionProps) { const { @@ -44,6 +46,7 @@ export function FilterCondition({ handleInputFocus, handleInputBlur, handleLabelClick, + handlePropertyChange, handleKeyDown, handleRemoveCondition, handleSelectMenuItem, @@ -55,10 +58,12 @@ export function FilterCondition({ const valueRef = useRef(null) const wrapperRef = useRef(null) const property = filterProperties.find((p) => p.name === condition.propertyName) + const propertyLabelRef = useRef(null) const [showValueCustom, setShowValueCustom] = useState(false) const [hasTypedOperator, setHasTypedOperator] = useState(false) const [hasTypedValue, setHasTypedValue] = useState(false) const [localValue, setLocalValue] = useState((condition.value ?? '').toString()) + const [propertySearchText, setPropertySearchText] = useState('') const conditionValue = (condition.value ?? '').toString() @@ -95,6 +100,43 @@ export function FilterCondition({ handleInputBlur ) + useEffect(() => { + if (!isPropertyActive) setPropertySearchText('') + }, [isPropertyActive]) + + const propertyItems = useMemo( + () => + buildPropertyChangeItems({ + filterProperties, + currentPropertyName: condition.propertyName, + inputValue: propertySearchText, + }), + [filterProperties, condition.propertyName, propertySearchText] + ) + + const { + highlightedIndex: propHighlightedIndex, + handleKeyDown: handlePropertyKeyDown, + reset: resetPropHighlight, + } = useHighlightNavigation( + propertyItems.length, + (index) => { + if (propertyItems[index]) { + handlePropertyChange(path, propertyItems[index].value) + } + }, + handleKeyDown + ) + + useEffect(() => { + if (!isPropertyActive) resetPropHighlight() + }, [isPropertyActive, resetPropHighlight]) + + const handlePropertyBlur = useDeferredBlur( + wrapperRef as React.RefObject, + handleInputBlur + ) + const operatorItems = useMemo( () => buildOperatorItems( @@ -218,19 +260,67 @@ export function FilterCondition({
- handleLabelClick(path)} - > - {property.label} - + 0}> + +
+ {isPropertyActive ? ( + setPropertySearchText(e.target.value)} + onBlur={handlePropertyBlur} + onKeyDown={handlePropertyKeyDown} + className="h-full border-none bg-transparent py-0 pl-2 pr-1 text-xs focus:outline-none focus:ring-0 focus:shadow-none focus-visible:ring-0 focus-visible:ring-offset-0 text-foreground-light w-full absolute left-0 top-0" + placeholder={property.label} + autoFocus + aria-label={`Change property from ${property.label}`} + data-testid={`filter-property-search-${property.name}`} + tabIndex={-1} + autoComplete="off" + data-1p-ignore + data-lpignore="true" + data-form-type="other" + /> + ) : null} + handleLabelClick(path)} + > + {property.label} + +
+ + e.preventDefault()} + onCloseAutoFocus={(e) => e.preventDefault()} + onInteractOutside={(e) => { + const target = e.target as Node + if (wrapperRef.current && !wrapperRef.current.contains(target)) { + handleInputBlur() + } + }} + > + handlePropertyChange(path, item.value)} + includeIcon={false} + /> + + 0}>
@@ -281,7 +371,7 @@ export function FilterCondition({ 0)}> -
+
} diff --git a/packages/ui-patterns/src/FilterBar/FilterGroup.tsx b/packages/ui-patterns/src/FilterBar/FilterGroup.tsx index 3341a4b44f6..0e2c1258c6f 100644 --- a/packages/ui-patterns/src/FilterBar/FilterGroup.tsx +++ b/packages/ui-patterns/src/FilterBar/FilterGroup.tsx @@ -47,7 +47,6 @@ export function FilterGroup({ group, path }: FilterGroupProps) { const isActive = activeInput?.type === 'group' && pathsEqual(path, activeInput.path) - // Reset local value when group freeform value is cleared useEffect(() => { if (freeformText === '') { setLocalFreeformValue('') @@ -84,6 +83,11 @@ export function FilterGroup({ group, path }: FilterGroupProps) { return activeInput.type === 'operator' && pathsEqual(conditionPath, activeInput.path) } + const isPropertyActiveForCondition = (conditionPath: number[]) => { + if (!activeInput) return false + return activeInput.type === 'property' && pathsEqual(conditionPath, activeInput.path) + } + const isConditionHighlighted = (conditionPath: number[]) => { if (!highlightedConditionPath) return false return pathsEqual(conditionPath, highlightedConditionPath) @@ -171,6 +175,7 @@ export function FilterGroup({ group, path }: FilterGroupProps) { path={currentPath} isActive={isConditionActive(currentPath)} isOperatorActive={isOperatorActive(currentPath)} + isPropertyActive={isPropertyActiveForCondition(currentPath)} isHighlighted={isConditionHighlighted(currentPath)} /> )} diff --git a/packages/ui-patterns/src/FilterBar/menuItems.ts b/packages/ui-patterns/src/FilterBar/menuItems.ts index a618e50ceb1..80f69fccc6b 100644 --- a/packages/ui-patterns/src/FilterBar/menuItems.ts +++ b/packages/ui-patterns/src/FilterBar/menuItems.ts @@ -81,6 +81,19 @@ export function buildPropertyItems(params: { return items } +export function buildPropertyChangeItems(params: { + filterProperties: FilterProperty[] + currentPropertyName: string + inputValue: string +}): MenuItem[] { + const { filterProperties, currentPropertyName, inputValue } = params + + return filterProperties + .filter((prop) => prop.name !== currentPropertyName) + .filter((prop) => prop.label.toLowerCase().includes(inputValue.toLowerCase())) + .map((prop) => ({ value: prop.name, label: prop.label })) +} + export function buildValueItems( activeInput: Extract | null, activeFilters: FilterGroup, diff --git a/packages/ui-patterns/src/FilterBar/types.ts b/packages/ui-patterns/src/FilterBar/types.ts index 1bfb082b65b..e1ba3837c83 100644 --- a/packages/ui-patterns/src/FilterBar/types.ts +++ b/packages/ui-patterns/src/FilterBar/types.ts @@ -130,6 +130,7 @@ export type ActiveInputState = | { type: 'value'; path: ConditionPath } | { type: 'operator'; path: ConditionPath } | { type: 'group'; path: ConditionPath } + | { type: 'property'; path: ConditionPath } | null export type KeyboardNavigationConfig = { @@ -140,3 +141,9 @@ export type KeyboardNavigationConfig = { highlightedConditionPath: ConditionPath | null setHighlightedConditionPath: (path: ConditionPath | null) => void } + +export type ResolvedPropertyChange = { + operator: string + value: string + focusTarget: 'operator' | 'value' +} diff --git a/packages/ui-patterns/src/FilterBar/useKeyboardNavigation.ts b/packages/ui-patterns/src/FilterBar/useKeyboardNavigation.ts index d30c7a69983..cbc96bb5b66 100644 --- a/packages/ui-patterns/src/FilterBar/useKeyboardNavigation.ts +++ b/packages/ui-patterns/src/FilterBar/useKeyboardNavigation.ts @@ -124,7 +124,7 @@ export function useKeyboardNavigation({ const handleBackspace = useCallback( (e: KeyboardEvent) => { - if (activeInput?.type === 'operator') return + if (activeInput?.type === 'operator' || activeInput?.type === 'property') return const inputElement = e.target as HTMLInputElement const isEmpty = inputElement.value === '' @@ -172,6 +172,8 @@ export function useKeyboardNavigation({ const handleArrowLeft = useCallback( (e: KeyboardEvent) => { + if (activeInput?.type === 'property') return + const inputElement = e.target as HTMLInputElement const isEmpty = inputElement.value === '' @@ -201,6 +203,8 @@ export function useKeyboardNavigation({ const handleArrowRight = useCallback( (e: KeyboardEvent) => { + if (activeInput?.type === 'property') return + const inputElement = e.target as HTMLInputElement const isEmpty = inputElement.value === '' @@ -255,7 +259,10 @@ export function useKeyboardNavigation({ } else if (e.key === 'ArrowRight') { handleArrowRight(e) } else if (e.key === 'Escape') { - if (highlightedConditionPath) { + if (activeInput?.type === 'property') { + e.preventDefault() + setActiveInput({ type: 'value', path: activeInput.path }) + } else if (highlightedConditionPath) { e.preventDefault() setHighlightedConditionPath(null) } else { diff --git a/packages/ui-patterns/src/FilterBar/utils.test.ts b/packages/ui-patterns/src/FilterBar/utils.test.ts index c3a2d8f7251..e67524d7b1f 100644 --- a/packages/ui-patterns/src/FilterBar/utils.test.ts +++ b/packages/ui-patterns/src/FilterBar/utils.test.ts @@ -1,6 +1,7 @@ import * as React from 'react' import { describe, expect, it } from 'vitest' +import { buildPropertyChangeItems } from './menuItems' import { FilterGroup, FilterProperty, MenuItem } from './types' import { addFilterToGroup, @@ -15,9 +16,11 @@ import { isFilterOptionObject, isSyncOptionsFunction, removeFromGroup, + resolvePropertyChange, truncateText, updateNestedLogicalOperator, updateNestedOperator, + updateNestedPropertyName, updateNestedValue, } from './utils' @@ -194,6 +197,141 @@ describe('FilterBar Utils', () => { }) }) + describe('updateNestedPropertyName', () => { + it('updates property name at root path', () => { + const result = updateNestedPropertyName(sampleFilterGroup, [0], 'email') + expect(result.conditions[0]).toEqual({ + propertyName: 'email', + value: 'test', + operator: '=', + }) + }) + + it('updates property name at nested path', () => { + const result = updateNestedPropertyName(sampleFilterGroup, [1, 0], 'role') + const nestedGroup = result.conditions[1] as FilterGroup + expect(nestedGroup.conditions[0]).toEqual({ + propertyName: 'role', + value: 'active', + operator: '=', + }) + }) + + it('updates property name and resets operator and value', () => { + const result = updateNestedPropertyName(sampleFilterGroup, [0], 'email', '', '') + expect(result.conditions[0]).toEqual({ + propertyName: 'email', + value: '', + operator: '', + }) + }) + + it('updates property name and preserves compatible operator', () => { + const result = updateNestedPropertyName(sampleFilterGroup, [0], 'email', '=', 'test') + expect(result.conditions[0]).toEqual({ + propertyName: 'email', + value: 'test', + operator: '=', + }) + }) + + it('does not modify other conditions', () => { + const result = updateNestedPropertyName(sampleFilterGroup, [0], 'email') + expect(result.conditions[1]).toEqual(sampleFilterGroup.conditions[1]) + }) + }) + + describe('resolvePropertyChange', () => { + const stringProperty: FilterProperty = { + label: 'Email', + name: 'email', + type: 'string', + operators: [ + { value: '=', label: 'Equals', group: 'comparison' }, + { value: '<>', label: 'Not equal', group: 'comparison' }, + { value: '~~', label: 'Like', group: 'pattern' }, + ], + } + + const booleanProperty: FilterProperty = { + label: 'Active', + name: 'active', + type: 'boolean', + options: [ + { label: 'true', value: 'true' }, + { label: 'false', value: 'false' }, + ], + operators: [ + { value: '=', label: 'Equals', group: 'comparison' }, + { value: '<>', label: 'Not equal', group: 'comparison' }, + ], + } + + const noOperatorsProperty: FilterProperty = { + label: 'Raw', + name: 'raw', + type: 'string', + } + + it('preserves compatible operator and free-form value', () => { + const result = resolvePropertyChange('=', 'hello', stringProperty) + expect(result).toEqual({ operator: '=', value: 'hello', focusTarget: 'value' }) + }) + + it('resets operator when incompatible with new property', () => { + const result = resolvePropertyChange('in', 'something', stringProperty) + expect(result).toEqual({ operator: '', value: '', focusTarget: 'operator' }) + }) + + it('preserves operator but resets value when fixed options do not match', () => { + const result = resolvePropertyChange('=', 'hello', booleanProperty) + expect(result).toEqual({ operator: '=', value: '', focusTarget: 'value' }) + }) + + it('preserves operator and value when fixed options match', () => { + const result = resolvePropertyChange('=', 'true', booleanProperty) + expect(result).toEqual({ operator: '=', value: 'true', focusTarget: 'value' }) + }) + + it('resets everything when current operator is empty', () => { + const result = resolvePropertyChange('', 'hello', stringProperty) + expect(result).toEqual({ operator: '', value: '', focusTarget: 'operator' }) + }) + + it('resets everything when new property has no operators', () => { + const result = resolvePropertyChange('=', 'hello', noOperatorsProperty) + expect(result).toEqual({ operator: '', value: '', focusTarget: 'operator' }) + }) + + it('preserves operator with empty value', () => { + const result = resolvePropertyChange('=', '', stringProperty) + expect(result).toEqual({ operator: '=', value: '', focusTarget: 'value' }) + }) + + it('handles string-based operators (not FilterOperatorObject)', () => { + const simpleOpProperty: FilterProperty = { + label: 'Simple', + name: 'simple', + type: 'string', + operators: ['=', '!='], + } + const result = resolvePropertyChange('=', 'test', simpleOpProperty) + expect(result).toEqual({ operator: '=', value: 'test', focusTarget: 'value' }) + }) + + it('handles string-based fixed options', () => { + const stringOptionsProperty: FilterProperty = { + label: 'Status', + name: 'status', + type: 'string', + options: ['active', 'inactive'], + operators: ['='], + } + const result = resolvePropertyChange('=', 'active', stringOptionsProperty) + expect(result).toEqual({ operator: '=', value: 'active', focusTarget: 'value' }) + }) + }) + describe('Type guards', () => { it('identifies custom option objects', () => { const customOption = { component: () => React.createElement('div', {}, 'test') } @@ -455,4 +593,61 @@ describe('FilterBar Utils', () => { expect(buildFilterPlaceholder(props, { maxProperties: 2 })).toBe('Filter by Name, Status') }) }) + + describe('buildPropertyChangeItems', () => { + const testProperties: FilterProperty[] = [ + { label: 'Name', name: 'name', type: 'string' }, + { label: 'Status', name: 'status', type: 'string' }, + { label: 'Count', name: 'count', type: 'number' }, + ] + + it('returns all properties except the current one', () => { + const items = buildPropertyChangeItems({ + filterProperties: testProperties, + currentPropertyName: 'name', + inputValue: '', + }) + expect(items).toHaveLength(2) + expect(items.map((i) => i.value)).toEqual(['status', 'count']) + }) + + it('filters by input value', () => { + const items = buildPropertyChangeItems({ + filterProperties: testProperties, + currentPropertyName: 'name', + inputValue: 'sta', + }) + expect(items).toHaveLength(1) + expect(items[0].value).toBe('status') + }) + + it('returns empty array when no matches', () => { + const items = buildPropertyChangeItems({ + filterProperties: testProperties, + currentPropertyName: 'name', + inputValue: 'xyz', + }) + expect(items).toHaveLength(0) + }) + + it('is case insensitive', () => { + const items = buildPropertyChangeItems({ + filterProperties: testProperties, + currentPropertyName: 'name', + inputValue: 'STA', + }) + expect(items).toHaveLength(1) + expect(items[0].value).toBe('status') + }) + + it('returns items with label and value', () => { + const items = buildPropertyChangeItems({ + filterProperties: testProperties, + currentPropertyName: 'count', + inputValue: '', + }) + expect(items[0]).toEqual({ value: 'name', label: 'Name' }) + expect(items[1]).toEqual({ value: 'status', label: 'Status' }) + }) + }) }) diff --git a/packages/ui-patterns/src/FilterBar/utils.ts b/packages/ui-patterns/src/FilterBar/utils.ts index 719ececa866..76cf3551a7c 100644 --- a/packages/ui-patterns/src/FilterBar/utils.ts +++ b/packages/ui-patterns/src/FilterBar/utils.ts @@ -12,6 +12,7 @@ import { isGroup, MenuItem, MenuItemGroup, + ResolvedPropertyChange, SyncOptionsFunction, } from './types' @@ -224,6 +225,54 @@ export function updateNestedOperator( } } +export function updateNestedPropertyName( + group: FilterGroup, + path: number[], + newPropertyName: string, + newOperator?: string, + newValue?: string +): FilterGroup { + return updateNestedFilter(group, path, (condition) => ({ + ...condition, + propertyName: newPropertyName, + ...(newOperator !== undefined ? { operator: newOperator } : {}), + ...(newValue !== undefined ? { value: newValue } : {}), + })) +} + +export function resolvePropertyChange( + currentOperator: string, + currentValue: string, + newProperty: FilterProperty +): ResolvedPropertyChange { + if (!currentOperator || !newProperty.operators) { + return { operator: '', value: '', focusTarget: 'operator' } + } + + const operatorValues = newProperty.operators.map((op) => + isFilterOperatorObject(op) ? op.value : op + ) + + if (!operatorValues.includes(currentOperator)) { + return { operator: '', value: '', focusTarget: 'operator' } + } + + if (!currentValue) { + return { operator: currentOperator, value: '', focusTarget: 'value' } + } + + if (newProperty.options && Array.isArray(newProperty.options)) { + // New property has fixed options — only preserve value if it's in the list + const optionValues = newProperty.options.map((opt) => + typeof opt === 'string' ? opt : 'value' in opt ? opt.value : '' + ) + const preservedValue = optionValues.includes(currentValue) ? currentValue : '' + return { operator: currentOperator, value: preservedValue, focusTarget: 'value' } + } + + return { operator: currentOperator, value: currentValue, focusTarget: 'value' } +} + export function updateNestedLogicalOperator(group: FilterGroup, path: number[]): FilterGroup { if (path.length === 0) { return {