This is an automated email from the ASF dual-hosted git repository.
pierrejeambrun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new b26e77632dd Collapse detail tabs that do not fit into a More menu
(#76742)
b26e77632dd is described below
commit b26e77632dd88ba787e8c0dd4f56877d655c1308
Author: Pierre Jeambrun <[email protected]>
AuthorDate: Mon Nov 9 19:03:48 2026 +0100
Collapse detail tabs that do not fit into a More menu (#76742)
* Collapse detail tabs that do not fit into a More menu
The Dag, run and task instance pages show up to ten tabs, plus any plugin
tabs, next to the grid or graph. When the details panel is narrow the labels
wrap and squeeze into an unreadable row, so the tabs that do not fit now go
into a menu while the active tab stays visible.
* Show an icon-only More trigger when the tabs show icons
Below 600px the tabs only show icons, so a "More" label next to them stands
out and takes the room of another tab.
* Drop styles the More menu does not need
The measuring copy cannot overflow from a zero-width box wherever it is
anchored, is already hidden from assistive technology by its visibility, and
only tabs that fit are shown, so none of these change the behaviour.
* Open the Details tab from the More menu in the task tries e2e test
At the desktop viewport the task instance tabs no longer all fit in the
row, so Details is reached through the More menu.
* Keep plugin sub-routes and keyboard navigation working with the More menu
A plugin's React app has its own sub-routes, on which its tab read as
inactive and was the first moved into the menu. The index tabs could not
be reached with the arrow keys once in the menu, and the row briefly
moved every tab into the menu before the panel was measured.
---
.../airflow/ui/public/i18n/locales/en/common.json | 1 +
.../ui/src/layouts/Details/NavTabs.test.tsx | 207 +++++++++++++++++++++
.../src/airflow/ui/src/layouts/Details/NavTabs.tsx | 182 ++++++++++++++----
.../airflow/ui/tests/e2e/specs/task-tries.spec.ts | 4 +-
4 files changed, 353 insertions(+), 41 deletions(-)
diff --git a/airflow-core/src/airflow/ui/public/i18n/locales/en/common.json
b/airflow-core/src/airflow/ui/public/i18n/locales/en/common.json
index 9d47b4b177d..7075f40fa72 100644
--- a/airflow-core/src/airflow/ui/public/i18n/locales/en/common.json
+++ b/airflow-core/src/airflow/ui/public/i18n/locales/en/common.json
@@ -197,6 +197,7 @@
},
"save": "Save"
},
+ "moreTabs": "More",
"nav": {
"admin": "Admin",
"assets": "Assets",
diff --git a/airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.test.tsx
b/airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.test.tsx
new file mode 100644
index 00000000000..bb5ca2cad59
--- /dev/null
+++ b/airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.test.tsx
@@ -0,0 +1,207 @@
+/*!
+ * 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 { ChakraProvider, defaultSystem } from "@chakra-ui/react";
+import "@testing-library/jest-dom/vitest";
+import { fireEvent, render, screen, within } from "@testing-library/react";
+import { MemoryRouter, Outlet, Route, Routes, useLocation } from
"react-router-dom";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useContainerWidth } from "src/utils";
+
+import { NavTabs, type NavTab } from "./NavTabs";
+
+vi.mock("src/utils", async () => {
+ const actual = await vi.importActual("src/utils");
+
+ return { ...actual, useContainerWidth: vi.fn() };
+});
+
+// Every tab, and the "More" trigger, measures 100px, except a tab labelled
"Wide".
+const TAB_WIDTH = 100;
+const WIDE_TAB_WIDTH = 300;
+
+const TABS: Array<NavTab> = Array.from({ length: 9 }, (_, index) => ({
+ label: `Tab ${index}`,
+ value: `tab${index}`,
+}));
+
+const CurrentPath = () => {
+ const { pathname, search } = useLocation();
+
+ return <div data-testid="current-path">{`${pathname}${search}`}</div>;
+};
+
+const renderTabs = (initialPath: string, tabs: Array<NavTab> = TABS) =>
+ render(
+ <ChakraProvider value={defaultSystem}>
+ <MemoryRouter initialEntries={[initialPath]}>
+ <Routes>
+ <Route
+ element={
+ <>
+ <NavTabs tabs={tabs} />
+ <Outlet />
+ </>
+ }
+ path="/dag"
+ >
+ <Route element={<CurrentPath />} path=":tab" />
+ <Route element={<CurrentPath />} path="plugin/:page/*" />
+ </Route>
+ </Routes>
+ </MemoryRouter>
+ </ChakraProvider>,
+ );
+
+const getRowLabels = () =>
+ screen
+ .getAllByRole("link")
+ .filter((link) => link.closest("[role=menu]") === null)
+ .map((link) => link.textContent);
+
+describe("NavTabs", () => {
+ beforeEach(() => {
+ vi.spyOn(HTMLElement.prototype,
"getBoundingClientRect").mockImplementation(function getRect(
+ this: HTMLElement,
+ ) {
+ return { width: this.textContent === "Wide" ? WIDE_TAB_WIDTH : TAB_WIDTH
} as DOMRect;
+ });
+ });
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("renders every tab in the row when they all fit", () => {
+ vi.mocked(useContainerWidth).mockReturnValue(TABS.length * TAB_WIDTH);
+
+ renderTabs("/dag/tab0");
+
+ expect(getRowLabels()).toEqual(TABS.map(({ label }) => label));
+ expect(screen.queryByTestId("nav-tabs-more")).not.toBeInTheDocument();
+ });
+
+ it("keeps every tab in the row until the panel has been measured", () => {
+ vi.mocked(useContainerWidth).mockReturnValue(0);
+
+ renderTabs("/dag/tab0");
+
+ expect(getRowLabels()).toEqual(TABS.map(({ label }) => label));
+ });
+
+ it("moves the tabs that do not fit to the More menu", async () => {
+ vi.mocked(useContainerWidth).mockReturnValue(700);
+
+ renderTabs("/dag/tab0");
+
+ // 700px leaves room for the "More" trigger and six tabs.
+ expect(getRowLabels()).toEqual(["Tab 0", "Tab 1", "Tab 2", "Tab 3", "Tab
4", "Tab 5"]);
+
+ fireEvent.click(screen.getByTestId("nav-tabs-more"));
+ const menu = await screen.findByRole("menu");
+
+ expect(
+ within(menu)
+ .getAllByRole("menuitem")
+ .map((item) => item.textContent),
+ ).toEqual(["Tab 6", "Tab 7", "Tab 8"]);
+
+ fireEvent.click(within(menu).getByText("Tab 7"));
+
+ expect(screen.getByTestId("current-path")).toHaveTextContent("/dag/tab7");
+ });
+
+ it("moves every tab after the first one that does not fit to the menu", ()
=> {
+ vi.mocked(useContainerWidth).mockReturnValue(450);
+
+ renderTabs("/dag/tab0", [
+ { label: "Tab 0", value: "tab0" },
+ { label: "Wide", value: "wide" },
+ { label: "Tab 2", value: "tab2" },
+ { label: "Tab 3", value: "tab3" },
+ ]);
+
+ // Tab 2 and Tab 3 would fit after Tab 0, but the row fills in order and
stops at Wide.
+ expect(getRowLabels()).toEqual(["Tab 0"]);
+ });
+
+ it("keeps a tab active through matchPaths in the row", () => {
+ vi.mocked(useContainerWidth).mockReturnValue(700);
+
+ renderTabs("/dag/logs", [...TABS.slice(0, 8), { label: "Tab 8",
matchPaths: ["logs"], value: "tab8" }]);
+
+ expect(getRowLabels()).toEqual(["Tab 0", "Tab 1", "Tab 2", "Tab 3", "Tab
4", "Tab 8"]);
+ });
+
+ it("keeps a plugin tab active on the plugin's sub-routes", () => {
+ vi.mocked(useContainerWidth).mockReturnValue(700);
+
+ renderTabs("/dag/plugin/my-view/details", [
+ ...TABS.slice(0, 8),
+ { label: "My View", value: "plugin/my-view" },
+ ]);
+
+ expect(getRowLabels()).toEqual(["Tab 0", "Tab 1", "Tab 2", "Tab 3", "Tab
4", "My View"]);
+ });
+
+ it("keeps the search of a tab opened from the menu", async () => {
+ vi.mocked(useContainerWidth).mockReturnValue(700);
+
+ renderTabs("/dag/tab0", [
+ ...TABS.slice(0, 8),
+ { label: "Tab 8", search: "?try_number=2", value: "tab8" },
+ ]);
+
+ fireEvent.click(screen.getByTestId("nav-tabs-more"));
+ fireEvent.click(within(await screen.findByRole("menu")).getByText("Tab
8"));
+
+
expect(screen.getByTestId("current-path")).toHaveTextContent("/dag/tab8?try_number=2");
+ });
+
+ it("keeps the active tab in the row when it would overflow", () => {
+ vi.mocked(useContainerWidth).mockReturnValue(700);
+
+ renderTabs("/dag/tab8");
+
+ expect(getRowLabels()).toEqual(["Tab 0", "Tab 1", "Tab 2", "Tab 3", "Tab
4", "Tab 8"]);
+ expect(within(screen.getByRole("link", { name: "Tab 8" })).getByText("Tab
8")).toHaveAttribute(
+ "aria-current",
+ "page",
+ );
+ });
+
+ it("shows icons and an icon-only More trigger in a narrow panel", async ()
=> {
+ vi.mocked(useContainerWidth).mockReturnValue(400);
+
+ renderTabs(
+ "/dag/tab0",
+ TABS.map((tab) => ({ ...tab, icon: <span>{`icon ${tab.value}`}</span>
})),
+ );
+
+ expect(getRowLabels()).toEqual(["icon tab0", "icon tab1", "icon tab2"]);
+ const more = screen.getByRole("button", { name: "moreTabs" });
+
+ expect(more).not.toHaveTextContent("moreTabs");
+
+ fireEvent.click(more);
+ const menu = await screen.findByRole("menu");
+
+ expect(within(menu).getByRole("menuitem", { name: "Tab 3"
})).toBeInTheDocument();
+ });
+});
diff --git a/airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx
b/airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx
index 6dec35c0bdb..7c472d163ee 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx
@@ -16,10 +16,14 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { useRef, type ReactNode } from "react";
+import { useLayoutEffect, useRef, useState, type ReactNode } from "react";
-import { Center, Flex } from "@chakra-ui/react";
-import { NavLink, useLocation, useResolvedPath } from "react-router-dom";
+import { Box, Center, type CenterProps, Flex, HStack } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+import { FiChevronDown, FiMoreHorizontal } from "react-icons/fi";
+import { NavLink, resolvePath, useLocation, useResolvedPath } from
"react-router-dom";
+
+import { Menu } from "src/system-components";
import { useContainerWidth } from "src/utils";
@@ -40,49 +44,99 @@ const INDICATOR_HEIGHT = "2px";
const trimTrailingSlash = (path: string) => path.replace(/\/$/u, "");
-const NavTabLink = ({
- containerWidth,
- icon,
- label,
- matchPaths,
- search,
- value,
-}: { readonly containerWidth: number } & NavTab) => {
+const TabContent = ({ active, children, ...rest }: { readonly active: boolean
} & CenterProps) => (
+ <Center
+ _focus={{ color: active ? "fg" : "brand.solid" }}
+ _hover={{ color: active ? "fg" : "brand.solid" }}
+ borderBottomColor={active ? "brand.solid" : "transparent"}
+ borderBottomWidth={INDICATOR_HEIGHT}
+ color={active ? "fg" : "fg.muted"}
+ fontSize="md"
+ fontWeight={active ? "bold" : "medium"}
+ height="40px"
+ mb={`-${INDICATOR_HEIGHT}`}
+ px={4}
+ transition="all 0.2s ease"
+ {...rest}
+ >
+ {children}
+ </Center>
+);
+
+export const NavTabs = ({ tabs }: Props) => {
+ const { t: translate } = useTranslation();
+ const containerRef = useRef<HTMLDivElement>(null);
+ const measureRef = useRef<HTMLDivElement>(null);
+ const containerWidth = useContainerWidth(containerRef);
+ const [widths, setWidths] = useState<{ more: number; tabs: Array<number>
}>({ more: 0, tabs: [] });
+
const { pathname } = useLocation();
- const resolved = useResolvedPath({ pathname: value, search });
+ const basePath = useResolvedPath(".").pathname;
// NavLink compares the raw location against its resolved target, and a run
id carries ":" and
// "+". Landing on a link where those arrived percent-encoded leaves the two
spellings of the
// same path looking different, so the tab for the page you are on reads as
inactive.
const current = trimTrailingSlash(decodeURIComponent(pathname));
- const target = trimTrailingSlash(decodeURIComponent(resolved.pathname));
const lastSegment = current.split("/").pop() ?? "";
- const active = current === target || (matchPaths ??
[]).includes(lastSegment);
+ const activeIndex = tabs.findIndex(({ matchPaths, value }) => {
+ const target = trimTrailingSlash(decodeURIComponent(resolvePath(value,
basePath).pathname));
- return (
- <NavLink end title={label} to={{ pathname: value, search }}>
- <Center
- _focus={{ color: active ? "fg" : "brand.solid" }}
- _hover={{ color: active ? "fg" : "brand.solid" }}
- aria-current={active ? "page" : undefined}
- borderBottomColor={active ? "brand.solid" : "transparent"}
- borderBottomWidth={INDICATOR_HEIGHT}
- color={active ? "fg" : "fg.muted"}
- fontSize="md"
- fontWeight={active ? "bold" : "medium"}
- height="40px"
- mb={`-${INDICATOR_HEIGHT}`}
- px={4}
- transition="all 0.2s ease"
- >
- {containerWidth > 600 || !Boolean(icon) ? label : icon}
- </Center>
- </NavLink>
+ // A plugin's React app is mounted at "plugin/:page/*", so its tab stays
active on the app's own
+ // sub-routes. Other tabs match exactly: the index tab's "" would
otherwise match every page.
+ return (
+ current === target ||
+ (value.startsWith("plugin/") && current.startsWith(`${target}/`)) ||
+ (matchPaths ?? []).includes(lastSegment)
+ );
+ });
+ const showLabels = containerWidth > 600;
+ const getTabContent = ({ icon, label }: NavTab) => (showLabels ||
!Boolean(icon) ? label : icon);
+ const moreLabel = showLabels ? (
+ <HStack as="span" gap={1}>
+ {translate("moreTabs")}
+ <FiChevronDown />
+ </HStack>
+ ) : (
+ <FiMoreHorizontal />
);
-};
-export const NavTabs = ({ tabs }: Props) => {
- const containerRef = useRef<HTMLDivElement>(null);
- const containerWidth = useContainerWidth(containerRef);
+ // Measure every tab in a hidden copy, including the ones moved to the menu.
The copy is rendered
+ // bold, as the active tab is, so a tab never grows once it is selected.
+ useLayoutEffect(() => {
+ const measured = [...(measureRef.current?.children ?? [])].map(
+ (child) => child.getBoundingClientRect().width,
+ );
+ const more = measured.pop() ?? 0;
+
+ setWidths((previous) =>
+ previous.more === more && previous.tabs.join(",") === measured.join(",")
+ ? previous
+ : { more, tabs: measured },
+ );
+ }, [tabs, showLabels]);
+
+ // Fill the row in order and move whatever does not fit behind "More". The
active tab always
+ // stays in the row, so the page being viewed is never hidden in the menu.
+ const visibleIndexes = new Set(tabs.map((_, index) => index));
+
+ // Until the panel has been measured its width is 0, and every tab would
move to the menu.
+ if (containerWidth > 0 && widths.tabs.reduce((total, width) => total +
width, 0) > containerWidth) {
+ let available = containerWidth - widths.more - (widths.tabs[activeIndex]
?? 0);
+ let isFull = false;
+
+ visibleIndexes.clear();
+ widths.tabs.forEach((width, index) => {
+ if (index === activeIndex) {
+ visibleIndexes.add(index);
+ } else if (!isFull && width <= available) {
+ available -= width;
+ visibleIndexes.add(index);
+ } else {
+ isFull = true;
+ }
+ });
+ }
+
+ const overflowTabs = tabs.filter((_, index) => !visibleIndexes.has(index));
return (
<Flex
@@ -92,9 +146,57 @@ export const NavTabs = ({ tabs }: Props) => {
mb={2}
ref={containerRef}
>
- {tabs.map((tab) => (
- <NavTabLink containerWidth={containerWidth} key={tab.value} {...tab} />
- ))}
+ {tabs.map((tab, index) =>
+ visibleIndexes.has(index) ? (
+ <NavLink end key={tab.value} title={tab.label} to={{ pathname:
tab.value, search: tab.search }}>
+ <TabContent
+ active={index === activeIndex}
+ aria-current={index === activeIndex ? "page" : undefined}
+ >
+ {getTabContent(tab)}
+ </TabContent>
+ </NavLink>
+ ) : undefined,
+ )}
+ {overflowTabs.length > 0 ? (
+ <Menu.Root positioning={{ placement: "bottom-end" }}>
+ <Menu.Trigger asChild>
+ <TabContent
+ active={false}
+ aria-label={translate("moreTabs")}
+ as="button"
+ cursor="pointer"
+ data-testid="nav-tabs-more"
+ title={translate("moreTabs")}
+ >
+ {moreLabel}
+ </TabContent>
+ </Menu.Trigger>
+ <Menu.Content>
+ {overflowTabs.map((tab) => (
+ // The index tabs' value is "", which the menu treats as no
item, so keyboard
+ // navigation could not move off them.
+ <Menu.Item asChild key={tab.value} value={`tab:${tab.value}`}>
+ <NavLink to={{ pathname: tab.value, search: tab.search }}>
+ {/* The label names the item; a plugin icon's alt text would
repeat it. */}
+ <span aria-hidden>{tab.icon}</span>
+ {tab.label}
+ </NavLink>
+ </Menu.Item>
+ ))}
+ </Menu.Content>
+ </Menu.Root>
+ ) : undefined}
+ <Box overflow="hidden" position="absolute" width={0}>
+ <Flex ref={measureRef} visibility="hidden" width="max-content">
+ {tabs.map((tab) => (
+ <TabContent active key={tab.value}>
+ {getTabContent(tab)}
+ </TabContent>
+ ))}
+ <TabContent active={false}>{moreLabel}</TabContent>
+ </Flex>
+ </Box>
</Flex>
);
};
diff --git a/airflow-core/src/airflow/ui/tests/e2e/specs/task-tries.spec.ts
b/airflow-core/src/airflow/ui/tests/e2e/specs/task-tries.spec.ts
index dfd0eace146..77209653d4f 100644
--- a/airflow-core/src/airflow/ui/tests/e2e/specs/task-tries.spec.ts
+++ b/airflow-core/src/airflow/ui/tests/e2e/specs/task-tries.spec.ts
@@ -84,7 +84,9 @@ test.describe("Mapped task tries", () => {
});
// Details and Logs describe the same try, so navigation and reload must
preserve try 2.
- await page.getByRole("link", { exact: true, name: "Details" }).click();
+ // At the desktop viewport the Details tab does not fit in the row and
sits behind "More".
+ await page.getByTestId("nav-tabs-more").click();
+ await page.getByRole("menuitem", { exact: true, name: "Details" }).click();
await expect(page).toHaveURL(/\/details\?try_number=2$/u);
await expect(
page.getByRole("row").filter({ has: page.getByRole("cell", { exact:
true, name: "State" }) }),
