diff --git a/airflow-core/src/airflow/ui/src/components/FilterBar/FilterBar.test.tsx b/airflow-core/src/airflow/ui/src/components/FilterBar/FilterBar.test.tsx new file mode 100644 index 0000000000000..d4469e41142ab --- /dev/null +++ b/airflow-core/src/airflow/ui/src/components/FilterBar/FilterBar.test.tsx @@ -0,0 +1,62 @@ +/*! + * 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 "@testing-library/jest-dom/vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +import { ChakraWrapper } from "src/utils/ChakraWrapper.tsx"; + +import { FilterBar } from "./FilterBar"; +import type { FilterConfig } from "./types"; + +const configs: Array = [ + { + key: "state", + label: "State", + type: "text", + }, +]; + +const getPillText = (value: string) => { + const matches = screen.getAllByText((_, element) => element?.textContent === `State: ${value}`); + + return matches[matches.length - 1]; +}; + +const queryPillText = (value: string) => + screen.queryAllByText((_, element) => element?.textContent === `State: ${value}`).at(-1); + +describe("FilterBar", () => { + it("updates existing pills when initial values change", async () => { + const onFiltersChange = vi.fn(); + const { rerender } = render( + , + { wrapper: ChakraWrapper }, + ); + + expect(getPillText("queued")).toBeInTheDocument(); + + rerender( + , + ); + + await waitFor(() => expect(getPillText("running")).toBeInTheDocument()); + expect(queryPillText("queued")).toBeUndefined(); + }); +}); diff --git a/airflow-core/src/airflow/ui/src/components/FilterBar/FilterBar.tsx b/airflow-core/src/airflow/ui/src/components/FilterBar/FilterBar.tsx index eed230bc98e86..a63f858842a3d 100644 --- a/airflow-core/src/airflow/ui/src/components/FilterBar/FilterBar.tsx +++ b/airflow-core/src/airflow/ui/src/components/FilterBar/FilterBar.tsx @@ -38,6 +38,14 @@ const defaultInitialValues: Record = {}; const getFilterIcon = (config: FilterConfig): React.ReactNode => config.icon ?? getDefaultFilterIcon(config.type); +const areFilterValuesEqual = (left: FilterValue, right: FilterValue) => { + try { + return JSON.stringify(left) === JSON.stringify(right); + } catch { + return Object.is(left, right); + } +}; + export const FilterBar = ({ configs, initialValues = defaultInitialValues, @@ -84,23 +92,40 @@ export const FilterBar = ({ } setFilters((prevFilters) => { - // Use a Set to avoid O(n²) lookup when checking for existing pills. - const existingKeys = new Set(prevFilters.map((filter) => filter.config.key)); - const toAdd = pillsToAdd.filter((pill) => !existingKeys.has(pill.config.key)); + const pillsByKey = new Map(pillsToAdd.map((pill) => [pill.config.key, pill])); - // Remove pills that had a committed value but whose URL param was cleared externally. - const afterRemove = prevFilters.filter((filter) => { - const pillHadValue = isValidFilterValue(filter.config.type, filter.value); - const urlValue = initialValues[filter.config.key]; + const reconciledFilters = prevFilters.flatMap((filter) => { + const pill = pillsByKey.get(filter.config.key); - return !pillHadValue || isValidFilterValue(filter.config.type, urlValue); + if (pill === undefined) { + const pillHadValue = isValidFilterValue(filter.config.type, filter.value); + const urlValue = initialValues[filter.config.key]; + + if (pillHadValue && !isValidFilterValue(filter.config.type, urlValue)) { + return []; + } + + return [filter]; + } + + if (areFilterValuesEqual(filter.value, pill.value)) { + return [filter]; + } + + return [{ ...filter, value: pill.value }]; }); - if (toAdd.length === 0 && afterRemove.length === prevFilters.length) { + const existingKeys = new Set(prevFilters.map((filter) => filter.config.key)); + const toAdd = pillsToAdd.filter((pill) => !existingKeys.has(pill.config.key)); + const hasReconciledChanges = + reconciledFilters.length !== prevFilters.length || + reconciledFilters.some((filter, index) => filter !== prevFilters[index]); + + if (!hasReconciledChanges && toAdd.length === 0) { return prevFilters; } - return [...afterRemove, ...toAdd]; + return [...reconciledFilters, ...toAdd]; }); // configs is intentionally omitted — it is structurally stable across renders and including // it would risk infinite re-render loops. initialValuesKey captures all relevant URL changes.