diff --git a/airflow-core/src/airflow/ui/src/components/DataTable/DataTable.test.tsx b/airflow-core/src/airflow/ui/src/components/DataTable/DataTable.test.tsx index ac8d5cf1ca5d6..83c2aaa1316ee 100644 --- a/airflow-core/src/airflow/ui/src/components/DataTable/DataTable.test.tsx +++ b/airflow-core/src/airflow/ui/src/components/DataTable/DataTable.test.tsx @@ -289,12 +289,12 @@ describe("DataTable", () => { expect(screen.getByRole("heading")).toHaveTextContent("50,000+ task"); }); - it("does not render row count heading during the initial load", () => { + it("keeps the row count heading during the initial load", () => { render(, { wrapper: ChakraWrapper, }); - expect(screen.queryByRole("heading")).toBeNull(); + expect(screen.getByRole("heading")).toHaveTextContent(/^task_other$/u); }); it("does not render row count heading when hideRowCountHeading is set", () => { @@ -469,7 +469,7 @@ describe("DataTable", () => { }); // Each slot needs its own entry in the header row condition, or its content vanishes whenever - // nothing else occupies the row — including while loading, when the row count is suppressed. + // nothing else occupies the row — as it does on tables that hide the row count heading. const actionSlots = [ ["filterActions", { filterActions: }], ["presentationActions", { presentationActions: }], @@ -482,14 +482,6 @@ describe("DataTable", () => { { wrapper: ChakraWrapper }, ); - expect(screen.getByText("slot content")).toBeInTheDocument(); - }); - - it.each(actionSlots)("keeps %s visible while loading", (_name, slot) => { - render(, { - wrapper: ChakraWrapper, - }); - expect(screen.queryByRole("heading")).toBeNull(); expect(screen.getByText("slot content")).toBeInTheDocument(); }); diff --git a/airflow-core/src/airflow/ui/src/components/DataTable/DataTable.tsx b/airflow-core/src/airflow/ui/src/components/DataTable/DataTable.tsx index 1cd5a43dbd97c..6e23b4312e8d3 100644 --- a/airflow-core/src/airflow/ui/src/components/DataTable/DataTable.tsx +++ b/airflow-core/src/airflow/ui/src/components/DataTable/DataTable.tsx @@ -206,8 +206,6 @@ export const DataTable = ({ (pageIndex !== 0 || rows.length !== rowTotal); const translateModelName = (count: number) => translate(modelName, { count }); - // During the initial load there is nothing to count yet - const showRowCount = !Boolean(hideRowCountHeading) && !Boolean(isLoading); const noRowsNode = noRowsMessage ?? translate("noItemsFound", { modelName: translateModelName(0) }); // i18next derives the plural form from the count, but in some languages (Russian) no integer count // ever selects `_other`, so read the count-free plural key directly instead of passing a stand-in. @@ -221,15 +219,18 @@ export const DataTable = ({ display === "table" && (showColumnsMenu ?? columns.length > 5) && table.getAllLeafColumns().some((column) => column.getCanHide()); - const headingNode = showRowCount ? ( + const hasRowCount = total !== undefined && !Boolean(isLoading); + // `filterActions` sits directly below this heading, so unmounting it while the count is unknown + // drags the filter controls up and drops them back once results arrive. + const headingNode = Boolean(hideRowCountHeading) ? undefined : ( - {total === undefined - ? pluralModelName - : `${total.toLocaleString(i18n.language)}${isCapped ? "+" : ""} ${translateModelName(total)}`} + {hasRowCount + ? `${total.toLocaleString(i18n.language)}${isCapped ? "+" : ""} ${translateModelName(total)}` + : pluralModelName} - ) : undefined; + ); // Every slot has to be listed here, or its content disappears whenever nothing else occupies - // the row — including every render where the row count is hidden while loading. + // the row — including tables that hide the row count heading entirely. const renderHeaderRow = headingNode !== undefined || headingExtra !== undefined || diff --git a/airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.test.tsx b/airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.test.tsx index 7763a9e92e345..0e15c6671fc40 100644 --- a/airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.test.tsx +++ b/airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.test.tsx @@ -17,11 +17,24 @@ * under the License. */ import "@testing-library/jest-dom"; -import { render, screen, waitFor } from "@testing-library/react"; -import { describe, expect, it } from "vitest"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { delay, http, HttpResponse } from "msw"; +import { setupServer, type SetupServer } from "msw/node"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { handlers } from "src/mocks/handlers"; import { AppWrapper } from "src/utils/AppWrapper"; +let server: SetupServer; + +beforeAll(() => { + server = setupServer(...handlers); + server.listen({ onUnhandledRequest: "bypass" }); +}); + +afterEach(() => server.resetHandlers()); +afterAll(() => server.close()); + // The assets mock handler (see src/mocks/handlers/assets.ts) returns a single asset // with one consuming task, one alias and one watcher. describe("AssetsList columns", () => { @@ -41,3 +54,26 @@ describe("AssetsList columns", () => { expect(screen.getByRole("button", { name: "1 watcher" })).toBeInTheDocument(); }); }); + +describe("AssetsList filtering", () => { + it("keeps the listed assets on screen while a filter change is still loading", async () => { + render(); + + await waitFor(() => expect(screen.getByText("asset_with_dependencies")).toBeInTheDocument()); + + server.use( + http.get("/ui/assets", async () => { + await delay("infinite"); + + return HttpResponse.json({ assets: [], total_entries: 0 }); + }), + ); + + fireEvent.change(screen.getByTestId("search-dags"), { target: { value: "plain" } }); + + await waitFor(() => expect(screen.getByRole("progressbar")).toBeVisible()); + + expect(screen.getByText("asset_with_dependencies")).toBeInTheDocument(); + expect(screen.queryAllByTestId("skeleton")).toHaveLength(0); + }); +}); diff --git a/airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.tsx b/airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.tsx index 9b1dfbcb9aef7..4bafdbb7c00c8 100644 --- a/airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.tsx +++ b/airflow-core/src/airflow/ui/src/pages/AssetsList/AssetsList.tsx @@ -152,15 +152,19 @@ export const AssetsList = () => { value: searchParams.get(SearchParamsKeys.GROUP_PATTERN), }); - const { data, error, isLoading } = useAssetServiceGetAssetsUi({ - ...groupArg, - lastAssetEventTimestampGte: lastAssetEventTimestampGte ?? undefined, - lastAssetEventTimestampLte: lastAssetEventTimestampLte ?? undefined, - limit: pagination.pageSize, - ...(advancedSearch.enabled ? { namePattern } : { namePrefixPattern: namePattern }), - offset: pagination.pageIndex * pagination.pageSize, - orderBy, - }); + const { data, error, isFetching, isLoading } = useAssetServiceGetAssetsUi( + { + ...groupArg, + lastAssetEventTimestampGte: lastAssetEventTimestampGte ?? undefined, + lastAssetEventTimestampLte: lastAssetEventTimestampLte ?? undefined, + limit: pagination.pageSize, + ...(advancedSearch.enabled ? { namePattern } : { namePrefixPattern: namePattern }), + offset: pagination.pageIndex * pagination.pageSize, + orderBy, + }, + undefined, + { placeholderData: (prev) => prev }, + ); const columns = createColumns(translate); const totalEntries = data?.total_entries ?? 0; @@ -200,6 +204,7 @@ export const AssetsList = () => { } initialState={tableURLState} + isFetching={isFetching} isLoading={isLoading} modelName="common:asset" onStateChange={setTableURLState} diff --git a/airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.test.tsx b/airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.test.tsx index 8b897411590a5..86410475547cd 100644 --- a/airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.test.tsx +++ b/airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.test.tsx @@ -18,12 +18,26 @@ */ import "@testing-library/jest-dom"; import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; -import { afterEach, describe, expect, it } from "vitest"; +import { delay, http, HttpResponse } from "msw"; +import { setupServer, type SetupServer } from "msw/node"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; import { DAGS_LIST_DISPLAY_KEY } from "src/constants/localStorage"; +import { handlers } from "src/mocks/handlers"; import { AppWrapper } from "src/utils/AppWrapper"; -afterEach(() => localStorage.clear()); +let server: SetupServer; + +beforeAll(() => { + server = setupServer(...handlers); + server.listen({ onUnhandledRequest: "bypass" }); +}); + +afterEach(() => { + server.resetHandlers(); + localStorage.clear(); +}); +afterAll(() => server.close()); describe("Dag Filters", () => { it("Filter by selected last run state", async () => { @@ -43,6 +57,32 @@ describe("Dag Filters", () => { await waitFor(() => screen.getByTestId("last_dag_run_state-filter-failed").click()); await waitFor(() => expect(screen.getByText("tutorial_taskflow_api_failed")).toBeInTheDocument()); }); + + it("keeps the listed Dags on screen while a newly added filter is still loading", async () => { + render(); + + await waitFor(() => expect(screen.getByText("tutorial_taskflow_api_failed")).toBeInTheDocument()); + + server.use( + http.get("/ui/dags", async () => { + await delay("infinite"); + + return HttpResponse.json({ dags: [], total_entries: 0 }); + }), + ); + + fireEvent.click(screen.getByTestId("add-filter-button")); + fireEvent.click(await screen.findByTestId("add-filter-last_dag_run_state")); + await waitFor(() => screen.getByTestId("last_dag_run_state-filter-success").click()); + + await waitFor(() => { + expect(screen.getByTestId("last_dag_run_state-pill")).toBeInTheDocument(); + expect(screen.getByRole("progressbar")).toBeVisible(); + }); + + expect(screen.getByText("tutorial_taskflow_api_failed")).toBeInTheDocument(); + expect(screen.queryAllByTestId("skeleton")).toHaveLength(0); + }); }); describe("Dag sorting", () => { diff --git a/airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.tsx b/airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.tsx index 34e7c3bdc1742..2d2a3888ad01b 100644 --- a/airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.tsx +++ b/airflow-core/src/airflow/ui/src/pages/DagsList/DagsList.tsx @@ -309,7 +309,7 @@ export const DagsList = () => { pendingHitl = false; } - const { data, error, isLoading } = useDags({ + const { data, error, isFetching, isLoading } = useDags({ advancedSearch: advancedSearch.enabled, dagDisplayNamePattern: Boolean(dagDisplayNamePattern) ? dagDisplayNamePattern : undefined, dagRunsLimit, @@ -377,6 +377,7 @@ export const DagsList = () => { } headingExtra={} initialState={tableURLState} + isFetching={isFetching} isLoading={isLoading} modelName="common:dag" onDisplayToggleChange={setDisplay} diff --git a/airflow-core/src/airflow/ui/src/queries/useDags.tsx b/airflow-core/src/airflow/ui/src/queries/useDags.tsx index 566f0a1d94b2d..d6c7ca7c721f3 100644 --- a/airflow-core/src/airflow/ui/src/queries/useDags.tsx +++ b/airflow-core/src/airflow/ui/src/queries/useDags.tsx @@ -84,6 +84,8 @@ export const useDags = ({ }, undefined, { + // Filter changes swap the query key, which would otherwise drop the list to skeletons + placeholderData: (prev) => prev, refetchInterval: (query) => refetchInterval === false ? false