From 4d95a8d0348ff6cbbe3d1e5ee6fa61ddef4e5ab2 Mon Sep 17 00:00:00 2001 From: Kasia <36897697+kasiazjc@users.noreply.github.com> Date: Sat, 30 May 2026 10:30:40 +0200 Subject: [PATCH] feat(listview): compact filter pills with popover for CRUD views (#40169) --- scripts/oxlint.sh | 2 +- .../spec/helpers/testing-library.tsx | 30 ++ .../src/components/Chart/chartAction.ts | 7 +- .../ListView/CardSortSelect.test.tsx | 102 +++++ .../components/ListView/CardSortSelect.tsx | 76 ++-- .../Filters/CompactFilterTrigger.test.tsx | 145 +++++++ .../ListView/Filters/CompactFilterTrigger.tsx | 198 +++++++++ .../Filters/CompactSelectPanel.test.tsx | 339 +++++++++++++++ .../ListView/Filters/CompactSelectPanel.tsx | 318 +++++++++++++++ .../components/ListView/Filters/DateRange.tsx | 112 ----- .../Filters/FilterPopoverContent.test.tsx | 80 ++++ .../ListView/Filters/FilterPopoverContent.tsx | 74 ++++ .../ListView/Filters/Select.test.tsx | 267 ------------ .../components/ListView/Filters/Select.tsx | 154 ------- .../ListView/Filters/TimeRange.test.tsx | 251 ++++++++++++ .../components/ListView/Filters/TimeRange.tsx | 291 +++++++++++++ .../ListView/Filters/index.test.tsx | 335 ++++++++++++++- .../src/components/ListView/Filters/index.tsx | 385 ++++++++++++++---- .../src/components/ListView/ListView.test.tsx | 21 +- .../src/components/ListView/ListView.tsx | 81 +++- .../src/pages/ChartList/ChartList.test.tsx | 9 +- .../DashboardList.cardview.test.tsx | 16 +- .../DashboardList/DashboardList.test.tsx | 8 +- .../DatasetList.integration.test.tsx | 6 +- .../DatasetList/DatasetList.listview.test.tsx | 23 +- .../pages/DatasetList/DatasetList.test.tsx | 7 +- .../src/pages/GroupsList/GroupsList.test.tsx | 16 +- .../src/pages/RolesList/RolesList.test.tsx | 7 +- .../RowLevelSecurityList.test.tsx | 9 +- .../src/pages/UsersList/UsersList.test.tsx | 20 +- 30 files changed, 2649 insertions(+), 740 deletions(-) create mode 100644 superset-frontend/src/components/ListView/CardSortSelect.test.tsx create mode 100644 superset-frontend/src/components/ListView/Filters/CompactFilterTrigger.test.tsx create mode 100644 superset-frontend/src/components/ListView/Filters/CompactFilterTrigger.tsx create mode 100644 superset-frontend/src/components/ListView/Filters/CompactSelectPanel.test.tsx create mode 100644 superset-frontend/src/components/ListView/Filters/CompactSelectPanel.tsx delete mode 100644 superset-frontend/src/components/ListView/Filters/DateRange.tsx create mode 100644 superset-frontend/src/components/ListView/Filters/FilterPopoverContent.test.tsx create mode 100644 superset-frontend/src/components/ListView/Filters/FilterPopoverContent.tsx delete mode 100644 superset-frontend/src/components/ListView/Filters/Select.test.tsx delete mode 100644 superset-frontend/src/components/ListView/Filters/Select.tsx create mode 100644 superset-frontend/src/components/ListView/Filters/TimeRange.test.tsx create mode 100644 superset-frontend/src/components/ListView/Filters/TimeRange.tsx diff --git a/scripts/oxlint.sh b/scripts/oxlint.sh index 95f48afabb6..9baf026ddf2 100755 --- a/scripts/oxlint.sh +++ b/scripts/oxlint.sh @@ -55,7 +55,7 @@ if [ ${#js_ts_files[@]} -gt 0 ]; then echo "$output" >&2 exit 1 } - [ -n "$output" ] && echo "$output" + if [ -n "$output" ]; then echo "$output"; fi else echo "No JavaScript/TypeScript files to lint" fi diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index c55f2991c0e..be4211bd72a 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -152,3 +152,33 @@ export async function selectOption(option: string, selectName?: string) { ); await userEvent.click(item); } + +/** + * Select an option from a compact pill filter (new UI that replaced comboboxes). + * Clicks the pill button matching the label, then clicks the option in the panel. + */ +export async function selectPillOption(option: string, pillLabel?: string) { + let pill: HTMLElement; + if (pillLabel) { + // Find the pill whose text content includes the label + pill = await waitFor(() => { + const pills = screen.getAllByTestId('compact-filter-pill'); + const match = pills.find(p => p.textContent?.includes(pillLabel)); + if (!match) + throw new Error(`Could not find pill with label "${pillLabel}"`); + return match; + }); + } else { + pill = await screen.findByTestId('compact-filter-pill'); + } + await userEvent.click(pill); + // Wait for the option list to appear and click the item + const item = await waitFor(() => { + const listbox = document.querySelector('[role="listbox"]'); + if (!listbox) throw new Error('No listbox found'); + const opt = within(listbox as HTMLElement).getByText(option); + if (!opt) throw new Error(`Option "${option}" not found`); + return opt; + }); + await userEvent.click(item); +} diff --git a/superset-frontend/src/components/Chart/chartAction.ts b/superset-frontend/src/components/Chart/chartAction.ts index c44c3ccd341..53caea51e88 100644 --- a/superset-frontend/src/components/Chart/chartAction.ts +++ b/superset-frontend/src/components/Chart/chartAction.ts @@ -817,8 +817,11 @@ export function exploreJSON( ), ); (queriesResponse as QueryData[]).forEach(response => { - if (response.warning) { - dispatch(addWarningToast(response.warning, { noDuplicate: true })); + const { warning } = response as QueryData & { + warning?: string | null; + }; + if (warning) { + dispatch(addWarningToast(warning, { noDuplicate: true })); } }); return dispatch( diff --git a/superset-frontend/src/components/ListView/CardSortSelect.test.tsx b/superset-frontend/src/components/ListView/CardSortSelect.test.tsx new file mode 100644 index 00000000000..6e00683999e --- /dev/null +++ b/superset-frontend/src/components/ListView/CardSortSelect.test.tsx @@ -0,0 +1,102 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import { CardSortSelect } from './CardSortSelect'; + +const options = [ + { desc: false, id: 'title', label: 'Alphabetical', value: 'alphabetical' }, + { + desc: true, + id: 'changed_on', + label: 'Recently modified', + value: 'recently_modified', + }, + { + desc: false, + id: 'changed_on', + label: 'Least recently modified', + value: 'least_recently_modified', + }, +]; + +test('pill always shows "Sort" label with no value suffix and no clear button', () => { + render( + , + ); + expect(screen.getByText('Sort')).toBeInTheDocument(); + expect(screen.queryByText(/sort.*alphabetical/i)).not.toBeInTheDocument(); + expect(screen.queryByTestId('compact-filter-clear')).not.toBeInTheDocument(); + expect(screen.getByTestId('compact-filter-pill')).toHaveAttribute( + 'aria-expanded', + 'false', + ); +}); + +test('no clear button even when a non-default sort is active', () => { + render( + , + ); + expect(screen.getByText('Sort')).toBeInTheDocument(); + expect(screen.queryByTestId('compact-filter-clear')).not.toBeInTheDocument(); +}); + +test('clicking a sort option from the panel calls onChange with the correct id and desc', async () => { + const onChange = jest.fn(); + render( + , + ); + + await userEvent.click(screen.getByTestId('compact-filter-pill')); + expect(screen.getByText('Recently modified')).toBeInTheDocument(); + + await userEvent.click(screen.getByText('Recently modified')); + + expect(onChange).toHaveBeenCalledWith([{ id: 'changed_on', desc: true }]); + // Pill label stays "Sort" — value is in tooltip, not the label + expect(screen.getByText('Sort')).toBeInTheDocument(); +}); + +test('selecting a different option from the panel calls onChange with correct args', async () => { + const onChange = jest.fn(); + render( + , + ); + + await userEvent.click(screen.getByTestId('compact-filter-pill')); + await userEvent.click(screen.getByText('Least recently modified')); + + expect(onChange).toHaveBeenCalledWith([{ id: 'changed_on', desc: false }]); +}); diff --git a/superset-frontend/src/components/ListView/CardSortSelect.tsx b/superset-frontend/src/components/ListView/CardSortSelect.tsx index 57617fc2f25..96c71f81103 100644 --- a/superset-frontend/src/components/ListView/CardSortSelect.tsx +++ b/superset-frontend/src/components/ListView/CardSortSelect.tsx @@ -16,20 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useMemo } from 'react'; +import { useRef, useState } from 'react'; import { t } from '@apache-superset/core/translation'; -import { styled } from '@apache-superset/core/theme'; -import { FormLabel, Select } from '@superset-ui/core/components'; -import { SELECT_WIDTH } from './utils'; +import type { SelectOption } from './types'; import { CardSortSelectOption, SortColumn } from './types'; - -const SortContainer = styled.div` - display: inline-flex; - font-size: ${({ theme }) => theme.fontSizeSM}px; - align-items: center; - text-align: left; - width: ${SELECT_WIDTH}px; -`; +import CompactFilterTrigger from './Filters/CompactFilterTrigger'; +import CompactSelectPanel from './Filters/CompactSelectPanel'; +import type { FilterHandler } from './Filters/types'; interface CardViewSelectSortProps { onChange: (value: SortColumn[]) => void; @@ -42,6 +35,8 @@ export const CardSortSelect = ({ onChange, options, }: CardViewSelectSortProps) => { + const panelRef = useRef(null); + const defaultSort = (initialSort && options.find( @@ -50,44 +45,41 @@ export const CardSortSelect = ({ )) || options[0]; - const [value, setValue] = useState({ + const [currentValue, setCurrentValue] = useState({ label: defaultSort.label, value: defaultSort.value, }); - const formattedOptions = useMemo( - () => options.map(option => ({ label: option.label, value: option.value })), - [options], - ); + const selectOptions = options.map(o => ({ label: o.label, value: o.value })); - const handleOnChange = (selected: { label: string; value: string }) => { - setValue(selected); - const originalOption = options.find( - ({ value }) => value === selected.value, - ); - if (originalOption) { - const sortBy = [ - { - id: originalOption.id, - desc: originalOption.desc, - }, - ]; - onChange(sortBy); + const handleSelect = (option: SelectOption | undefined) => { + if (!option) return; + const original = options.find(o => o.value === option.value); + if (original) { + setCurrentValue({ label: original.label, value: original.value }); + onChange([{ id: original.id, desc: original.desc }]); } }; return ( - - + } + placeholder={t('Search')} + value={search} + onChange={e => { + setSearch(e.target.value); + debouncedSetSearch(e.target.value); + }} + allowClear + css={css` + width: 100%; + box-shadow: none; + `} + /> + + )} + + {isLoading ? ( + {t('Loading...')} + ) : displayOptions.length === 0 ? ( + {t('No results')} + ) : ( + displayOptions.map((opt, i) => { + const isActive = value?.value === opt.value; + const getDisplayText = (el: HTMLElement) => + el.textContent?.trim() || undefined; + const isFirst = i === 0; + const isLast = i === displayOptions.length - 1; + return ( + + handleSelect(opt, getDisplayText(e.currentTarget)) + } + onKeyDown={e => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + handleSelect(opt, getDisplayText(e.currentTarget)); + } else if (e.key === 'ArrowDown' && !isLast) { + e.preventDefault(); + ( + e.currentTarget.nextElementSibling as HTMLElement | null + )?.focus(); + } else if (e.key === 'ArrowUp' && !isFirst) { + e.preventDefault(); + ( + e.currentTarget + .previousElementSibling as HTMLElement | null + )?.focus(); + } + }} + > + {opt.label} + {isActive && ( + + )} + + ); + }) + )} + + + ); +} + +export default forwardRef(CompactSelectPanel); diff --git a/superset-frontend/src/components/ListView/Filters/DateRange.tsx b/superset-frontend/src/components/ListView/Filters/DateRange.tsx deleted file mode 100644 index 1b735a5c9b0..00000000000 --- a/superset-frontend/src/components/ListView/Filters/DateRange.tsx +++ /dev/null @@ -1,112 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import { - useState, - useMemo, - forwardRef, - useImperativeHandle, - RefObject, -} from 'react'; - -import { t } from '@apache-superset/core/translation'; -import { Dayjs } from 'dayjs'; -import { useLocale } from 'src/hooks/useLocale'; -import { extendedDayjs } from '@superset-ui/core/utils/dates'; -import { - AntdThemeProvider, - Loading, - FormLabel, - RangePicker, -} from '@superset-ui/core/components'; -import type { BaseFilter, FilterHandler } from './types'; -import { FilterContainer } from './Base'; -import { RANGE_WIDTH } from '../utils'; - -interface DateRangeFilterProps extends BaseFilter { - onSubmit: (val: number[] | string[]) => void; - name: string; - dateFilterValueType?: 'unix' | 'iso'; -} - -type ValueState = [number, number] | [string, string] | null; - -function DateRangeFilter( - { - Header, - initialValue, - onSubmit, - dateFilterValueType = 'unix', - }: DateRangeFilterProps, - ref: RefObject, -) { - const [value, setValue] = useState(initialValue ?? null); - const dayjsValue = useMemo((): [Dayjs, Dayjs] | null => { - if (!value || (Array.isArray(value) && !value.length)) return null; - return [extendedDayjs(value[0]), extendedDayjs(value[1])]; - }, [value]); - - const locale = useLocale(); - - useImperativeHandle(ref, () => ({ - clearFilter: () => { - setValue(null); - onSubmit([]); - }, - })); - - if (locale === null) { - return ; - } - return ( - - - {Header} - { - if (!dayjsRange?.[0]?.valueOf() || !dayjsRange?.[1]?.valueOf()) { - setValue(null); - onSubmit([]); - return; - } - const changeValue = - dateFilterValueType === 'iso' - ? [dayjsRange[0].toISOString(), dayjsRange[1].toISOString()] - : [ - dayjsRange[0]?.valueOf() ?? 0, - dayjsRange[1]?.valueOf() ?? 0, - ]; - setValue(changeValue as ValueState); - onSubmit(changeValue); - }} - /> - - - ); -} - -export default forwardRef(DateRangeFilter); diff --git a/superset-frontend/src/components/ListView/Filters/FilterPopoverContent.test.tsx b/superset-frontend/src/components/ListView/Filters/FilterPopoverContent.test.tsx new file mode 100644 index 00000000000..f863f78d498 --- /dev/null +++ b/superset-frontend/src/components/ListView/Filters/FilterPopoverContent.test.tsx @@ -0,0 +1,80 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import FilterPopoverContent from './FilterPopoverContent'; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +test('renders children inside the wrapper', () => { + render( + +
Inner content
+ , + ); + expect(screen.getByTestId('inner-content')).toBeInTheDocument(); +}); + +test('renders the Apply button', () => { + render( + +
content
+ , + ); + expect(screen.getByRole('button', { name: /apply/i })).toBeInTheDocument(); +}); + +test('calls onClose when Apply button is clicked', async () => { + const onClose = jest.fn(); + render( + +
content
+ , + ); + await userEvent.click(screen.getByRole('button', { name: /apply/i })); + expect(onClose).toHaveBeenCalledTimes(1); +}); + +test('renders without onClose and clicking Apply does not throw', async () => { + render( + +
content
+ , + ); + // No onClose prop — click should not throw + await userEvent.click(screen.getByRole('button', { name: /apply/i })); + expect(screen.getByRole('button', { name: /apply/i })).toBeInTheDocument(); +}); + +test('visually hides label elements so pills remain accessible', () => { + render( + + + + , + ); + const label = screen.getByText('Date range'); + // The label must be in the DOM for screen readers but visually hidden via CSS + expect(label).toBeInTheDocument(); + const computedStyle = window.getComputedStyle(label); + // clip / overflow hidden pattern applied; position absolute is the key indicator + expect(computedStyle.position).toBe('absolute'); +}); diff --git a/superset-frontend/src/components/ListView/Filters/FilterPopoverContent.tsx b/superset-frontend/src/components/ListView/Filters/FilterPopoverContent.tsx new file mode 100644 index 00000000000..4d9ea3ae032 --- /dev/null +++ b/superset-frontend/src/components/ListView/Filters/FilterPopoverContent.tsx @@ -0,0 +1,74 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import type { ReactNode } from 'react'; +import { t } from '@apache-superset/core/translation'; +import { styled, css } from '@apache-superset/core/theme'; +import { Button } from '@superset-ui/core/components'; + +interface FilterPopoverContentProps { + children: ReactNode; + onClose?: () => void; +} + +const Wrapper = styled.div` + ${({ theme }) => css` + padding: ${theme.sizeUnit * 2}px; + display: flex; + flex-direction: column; + gap: ${theme.sizeUnit * 2}px; + background: ${theme.colorBgElevated}; + border-radius: ${theme.borderRadiusLG}px; + box-shadow: ${theme.boxShadowSecondary}; + + /* Visually hide the redundant label — the pill already shows it, but keep it + accessible to screen readers so filter inputs have a named context. */ + label { + position: absolute !important; + width: 1px; + height: 1px; + padding: 0; + margin: -1px; + overflow: hidden; + clip: rect(0, 0, 0, 0); + white-space: nowrap; + border: 0; + } + `} +`; + +const Footer = styled.div` + display: flex; + justify-content: flex-end; +`; + +export default function FilterPopoverContent({ + children, + onClose, +}: FilterPopoverContentProps) { + return ( + + {children} +
+ +
+ + ); +} diff --git a/superset-frontend/src/components/ListView/Filters/Select.test.tsx b/superset-frontend/src/components/ListView/Filters/Select.test.tsx deleted file mode 100644 index a0bedb32f20..00000000000 --- a/superset-frontend/src/components/ListView/Filters/Select.test.tsx +++ /dev/null @@ -1,267 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import { createRef } from 'react'; -import { - render, - screen, - selectOption, - waitFor, -} from 'spec/helpers/testing-library'; -import { ListViewFilterOperator } from '../types'; -import UIFilters from './index'; -import SelectFilter from './Select'; -import type { FilterHandler } from './types'; - -const mockUpdateFilterValue = jest.fn(); - -beforeEach(() => { - mockUpdateFilterValue.mockClear(); -}); - -test('select filter with ReactNode label uses option title when serializing selection', async () => { - // Regression for sc-104554: the chart-list Owner filter renders options - // with ReactNode labels (name + email). The value passed to - // updateFilterValue is serialized into URL / filter state and re-used to - // render the filter pill on return. It must carry the plain-text name - // (from `title`) and not fall back to the numeric user id. - const ReactNodeLabel = ( -
- John Doe - john@example.com -
- ); - - const fetchSelects = jest.fn().mockResolvedValue({ - data: [ - { - label: ReactNodeLabel, - value: 42, - title: 'John Doe', - }, - ], - totalCount: 1, - }); - - const filters = [ - { - Header: 'Owner', - key: 'owner', - id: 'owners', - input: 'select' as const, - operator: ListViewFilterOperator.RelationManyMany, - unfilteredLabel: 'All', - fetchSelects, - paginate: true, - }, - ]; - - render( - , - ); - - await selectOption('John Doe', 'Owner'); - - await waitFor(() => { - expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, { - label: 'John Doe', - value: 42, - }); - }); -}); - -test('select filter falls back to stringified value when no string label or title is available', async () => { - const fetchSelects = jest.fn().mockResolvedValue({ - data: [ - { - label: 123, - value: 123, - }, - ], - totalCount: 1, - }); - - const filters = [ - { - Header: 'Something', - key: 'something', - id: 'something', - input: 'select' as const, - operator: ListViewFilterOperator.RelationOneMany, - unfilteredLabel: 'All', - fetchSelects, - }, - ]; - - render( - , - ); - - await selectOption('123', 'Something'); - - await waitFor(() => { - expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, { - label: '123', - value: 123, - }); - }); -}); - -test('plain select with string label passes label through unchanged', async () => { - // Happy-path coverage for the typeof-string branch in onChange, exercised - // through the non-async Select wrapper (selects array, no fetchSelects). - const filters = [ - { - Header: 'Status', - key: 'status', - id: 'status', - input: 'select' as const, - operator: ListViewFilterOperator.Equals, - unfilteredLabel: 'All', - selects: [ - { label: 'Published', value: 7 }, - { label: 'Draft', value: 8 }, - ], - }, - ]; - - render( - , - ); - - await selectOption('Published', 'Status'); - - await waitFor(() => { - expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, { - label: 'Published', - value: 7, - }); - }); -}); - -test('plain select with ReactNode label uses option title when serializing selection', async () => { - // Parallel coverage to the AsyncSelect ReactNode-with-title test, against - // the non-async Select wrapper. Guards against the two wrappers ever - // diverging on antd's two-arg onChange shape. - const ReactNodeLabel = ( -
- Jane Roe - jane@example.com -
- ); - - const filters = [ - { - Header: 'Owner', - key: 'owner', - id: 'owners', - input: 'select' as const, - operator: ListViewFilterOperator.RelationManyMany, - unfilteredLabel: 'All', - selects: [{ label: ReactNodeLabel, value: 99, title: 'Jane Roe' }], - }, - ]; - - render( - , - ); - - await selectOption('Jane Roe', 'Owner'); - - await waitFor(() => { - expect(mockUpdateFilterValue).toHaveBeenCalledWith(0, { - label: 'Jane Roe', - value: 99, - }); - }); -}); - -test('clearFilter notifies onSelect with undefined and isClear=true', () => { - // The isClear flag is what allows the parent (Filters/index) to suppress - // onFilterUpdate side-effects when the user clears the filter rather than - // picking a new value. Lock that contract in. - const mockOnSelect = jest.fn(); - const ref = createRef(); - - render( - , - ); - - ref.current?.clearFilter(); - - expect(mockOnSelect).toHaveBeenCalledWith(undefined, true); -}); - -test('rehydrates filter pill from initialValue with plain-string label', async () => { - // The user-visible regression: after URL/state rehydration the filter pill - // must render the human-readable name, not the numeric user id. The fix - // ensures the persisted label is a string; this test asserts that string - // is what surfaces in the rendered combobox selection. - const filters = [ - { - Header: 'Owner', - key: 'owner', - id: 'owners', - input: 'select' as const, - operator: ListViewFilterOperator.RelationManyMany, - unfilteredLabel: 'All', - fetchSelects: jest.fn().mockResolvedValue({ data: [], totalCount: 0 }), - paginate: true, - }, - ]; - - render( - , - ); - - await waitFor(() => { - expect(screen.getByText('John Doe')).toBeInTheDocument(); - }); -}); diff --git a/superset-frontend/src/components/ListView/Filters/Select.tsx b/superset-frontend/src/components/ListView/Filters/Select.tsx deleted file mode 100644 index cecee38a274..00000000000 --- a/superset-frontend/src/components/ListView/Filters/Select.tsx +++ /dev/null @@ -1,154 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import { - useState, - useMemo, - forwardRef, - useImperativeHandle, - type RefObject, -} from 'react'; - -import { t } from '@apache-superset/core/translation'; -import { Select, AsyncSelect, FormLabel } from '@superset-ui/core/components'; -import { ListViewFilter as Filter, SelectOption } from '../types'; -import type { BaseFilter, FilterHandler } from './types'; -import { FilterContainer } from './Base'; -import { SELECT_WIDTH } from '../utils'; - -interface SelectFilterProps extends BaseFilter { - fetchSelects?: Filter['fetchSelects']; - name?: string; - onSelect: (selected: SelectOption | undefined, isClear?: boolean) => void; - optionFilterProps?: string[]; - paginate?: boolean; - selects: Filter['selects']; - loading?: boolean; - dropdownStyle?: React.CSSProperties; -} - -function SelectFilter( - { - Header, - name, - fetchSelects, - initialValue, - onSelect, - optionFilterProps, - selects = [], - loading = false, - dropdownStyle, - }: SelectFilterProps, - ref: RefObject, -) { - const [selectedOption, setSelectedOption] = useState(initialValue); - - const onChange = (selected: SelectOption, option?: SelectOption) => { - // antd's `onChange` (with `labelInValue`) passes the `{label, value}` - // labeled-value as the first arg and the full option (which carries - // `title` and any other fields) as the second. Options may supply a - // ReactNode label (e.g. OwnerSelectLabel for the chart list Owner - // filter). Since this object is serialized into the URL and rehydrated - // as the filter pill on return, we need a plain string. Prefer `title` - // (set by callers to the human-readable name) before falling back to - // the value. - onSelect( - selected - ? { - label: - typeof selected.label === 'string' - ? selected.label - : (option?.title ?? String(selected.value)), - value: selected.value, - } - : undefined, - ); - setSelectedOption(selected); - }; - - const onClear = () => { - onSelect(undefined, true); - setSelectedOption(undefined); - }; - - useImperativeHandle(ref, () => ({ - clearFilter: () => { - onClear(); - }, - })); - - const fetchAndFormatSelects = useMemo( - () => async (inputValue: string, page: number, pageSize: number) => { - if (fetchSelects) { - const selectValues = await fetchSelects(inputValue, page, pageSize); - return { - data: selectValues.data, - totalCount: selectValues.totalCount, - }; - } - return { - data: [], - totalCount: 0, - }; - }, - [fetchSelects], - ); - const placeholder = t('Choose...'); - return ( - - {Header} - {fetchSelects ? ( - - ) : ( -