This is an automated email from the ASF dual-hosted git repository.
bbovenzi 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 b9c657b3bcd Make Dags sidebar button a link and move Dag Runs and Task
Instances to Browse (#75422)
b9c657b3bcd is described below
commit b9c657b3bcd73c46f803d04a66d0d9c1280db6ec
Author: Stephen Yang <[email protected]>
AuthorDate: Sat Oct 24 02:43:38 2026 +0800
Make Dags sidebar button a link and move Dag Runs and Task Instances to
Browse (#75422)
The Dags button opened a hover menu, so clicking it never reached the Dags
page. Make it a plain link to /dags and list Dag Runs and Task Instances in the
Browse menu, gated by the Dags permission.
---
.../ui/src/layouts/Nav/BrowseButton.test.tsx | 83 ++++++++++++++++++++++
.../airflow/ui/src/layouts/Nav/BrowseButton.tsx | 32 +++++++--
.../airflow/ui/src/layouts/Nav/DagsButton.test.tsx | 54 +++++---------
.../src/airflow/ui/src/layouts/Nav/DagsButton.tsx | 41 ++---------
4 files changed, 129 insertions(+), 81 deletions(-)
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.test.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.test.tsx
new file mode 100644
index 00000000000..60d410bdc58
--- /dev/null
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.test.tsx
@@ -0,0 +1,83 @@
+/*!
+ * 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 { PropsWithChildren } from "react";
+
+import "@testing-library/jest-dom/vitest";
+import { fireEvent, render, screen, waitFor } from "@testing-library/react";
+import { MemoryRouter } from "react-router-dom";
+import { describe, expect, it } from "vitest";
+
+import { BaseWrapper, Wrapper } from "src/utils/Wrapper";
+
+import { BrowseButton } from "./BrowseButton";
+
+const getTrigger = () => screen.getByRole("button", { name: "nav.browse" });
+
+const openMenu = async () => {
+ fireEvent.click(getTrigger());
+ await waitFor(() =>
expect(screen.getAllByRole("menuitem").length).toBeGreaterThan(0));
+};
+
+const hrefs = () => screen.getAllByRole("menuitem").map((item) =>
item.getAttribute("href"));
+
+const wrapperAt = (path: string) => {
+ const wrapper = ({ children }: PropsWithChildren) => (
+ <BaseWrapper>
+ <MemoryRouter initialEntries={[path]}>{children}</MemoryRouter>
+ </BaseWrapper>
+ );
+
+ return wrapper;
+};
+
+describe("BrowseButton", () => {
+ it("lists Dag Runs and Task Instances for users who can see Dags", async ()
=> {
+ render(<BrowseButton authorizedMenuItems={["Dags"]} externalViews={[]} />,
{ wrapper: Wrapper });
+
+ await openMenu();
+
+ expect(hrefs()).toStrictEqual(["/dag_runs", "/task_instances"]);
+ });
+
+ it("hides Dag Runs and Task Instances without access to Dags", async () => {
+ render(<BrowseButton authorizedMenuItems={["Jobs"]} externalViews={[]} />,
{ wrapper: Wrapper });
+
+ await openMenu();
+
+ expect(hrefs()).toStrictEqual(["/jobs"]);
+ });
+
+ it("keeps the entries in alphabetical order", async () => {
+ render(<BrowseButton authorizedMenuItems={["Dags", "Jobs", "XComs"]}
externalViews={[]} />, {
+ wrapper: Wrapper,
+ });
+
+ await openMenu();
+
+ expect(hrefs()).toStrictEqual(["/dag_runs", "/jobs", "/task_instances",
"/xcoms"]);
+ });
+
+ it.each(["/dag_runs", "/task_instances"])("is highlighted on %s", (path) => {
+ render(<BrowseButton authorizedMenuItems={["Dags"]} externalViews={[]} />,
{
+ wrapper: wrapperAt(path),
+ });
+
+ expect(getTrigger()).toHaveAttribute("aria-current", "page");
+ });
+});
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx
index 7667d1b636a..c23f1f3f9fb 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx
@@ -42,6 +42,14 @@ const links = [
// Must stay byte-identical to MenuItem.DAG_BUNDLES: the filter below
matches on this string.
title: "Dag Bundles",
},
+ {
+ href: "/dag_runs",
+ key: "dagRuns",
+ // Reuses the label the Dags pages already show on their tabs.
+ labelKey: "dagRun_other",
+ // There is no menu item of its own for Dag runs: they follow the Dags
permission.
+ title: "Dags",
+ },
{
href: "/deadlines",
key: "deadlines",
@@ -52,6 +60,12 @@ const links = [
key: "jobs",
title: "Jobs",
},
+ {
+ href: "/task_instances",
+ key: "taskInstances",
+ labelKey: "taskInstance_other",
+ title: "Dags",
+ },
{
href: "/xcoms",
key: "xcoms",
@@ -73,13 +87,17 @@ export const BrowseButton = ({
}) => {
const { t: translate } = useTranslation();
const authorizedLinks = links.filter(({ title }) =>
authorizedMenuItems.includes(title as MenuItem));
- const menuItems = authorizedLinks.map((link) => (
- <Menu.Item asChild key={link.key} value={translate(`browse.${link.key}`)}>
- <RouterLink aria-label={translate(`browse.${link.key}`)} to={link.href}>
- {translate(`browse.${link.key}`)}
- </RouterLink>
- </Menu.Item>
- ));
+ const menuItems = authorizedLinks.map((link) => {
+ const label = translate(link.labelKey ?? `browse.${link.key}`);
+
+ return (
+ <Menu.Item asChild key={link.key} value={label}>
+ <RouterLink aria-label={label} to={link.href}>
+ {label}
+ </RouterLink>
+ </Menu.Item>
+ );
+ });
if (!menuItems.length && !externalViews.length) {
return undefined;
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.test.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.test.tsx
index b1146cd4eb4..42af91ba6d4 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.test.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.test.tsx
@@ -19,7 +19,7 @@
import type { PropsWithChildren } from "react";
import "@testing-library/jest-dom/vitest";
-import { fireEvent, render, screen, waitFor } from "@testing-library/react";
+import { fireEvent, render, screen } from "@testing-library/react";
import { MemoryRouter } from "react-router-dom";
import { describe, expect, it } from "vitest";
@@ -27,7 +27,7 @@ import { BaseWrapper, Wrapper } from "src/utils/Wrapper";
import { DagsButton } from "./DagsButton";
-const getTrigger = () => screen.getByRole("button", { name: "nav.dags" });
+const getLink = () => screen.getByRole("link", { name: "nav.dags" });
const wrapperAt = (path: string) => {
const wrapper = ({ children }: PropsWithChildren) => (
@@ -40,56 +40,36 @@ const wrapperAt = (path: string) => {
};
describe("DagsButton", () => {
- it("opens the menu on click instead of navigating", async () => {
+ it("is a link to the Dags page", () => {
render(<DagsButton authorizedMenuItems={["Dags"]} />, { wrapper: Wrapper
});
- expect(getTrigger()).not.toHaveAttribute("href");
-
- fireEvent.click(getTrigger());
-
- await waitFor(() => {
- expect(screen.getAllByRole("menuitem")).toHaveLength(3);
- });
+ expect(getLink()).toHaveAttribute("href", "/dags");
});
- it.each(["/dags", "/dags/example_dag/runs", "/dag_runs", "/task_instances"])(
- "is highlighted on %s",
- (path) => {
- render(<DagsButton authorizedMenuItems={["Dags"]} />, { wrapper:
wrapperAt(path) });
-
- expect(getTrigger()).toHaveAttribute("aria-current", "page");
- },
- );
-
- it("reveals the dags, dag runs and task instances links on hover", async ()
=> {
- render(<DagsButton authorizedMenuItems={["Dags"]} />, { wrapper: Wrapper
});
-
- expect(screen.queryByRole("menuitem")).toBeNull();
+ it.each(["/dags", "/dags/example_dag/runs"])("is highlighted on %s", (path)
=> {
+ render(<DagsButton authorizedMenuItems={["Dags"]} />, { wrapper:
wrapperAt(path) });
- fireEvent.pointerEnter(getTrigger(), { pointerType: "mouse" });
+ expect(getLink()).toHaveAttribute("aria-current", "page");
+ });
- await waitFor(() => {
- expect(screen.getAllByRole("menuitem")).toHaveLength(3);
- });
+ it.each(["/dag_runs", "/task_instances"])("is not highlighted on %s", (path)
=> {
+ render(<DagsButton authorizedMenuItems={["Dags"]} />, { wrapper:
wrapperAt(path) });
- expect(screen.getAllByRole("menuitem").map((item) =>
item.getAttribute("href"))).toStrictEqual([
- "/dags",
- "/dag_runs",
- "/task_instances",
- ]);
+ expect(getLink()).not.toHaveAttribute("aria-current");
});
- it("ignores hover from a touch pointer", async () => {
+ it("does not open a menu on hover", () => {
render(<DagsButton authorizedMenuItems={["Dags"]} />, { wrapper: Wrapper
});
- fireEvent.pointerEnter(getTrigger(), { pointerType: "touch" });
+ fireEvent.pointerEnter(getLink(), { pointerType: "mouse" });
- await expect(waitFor(() =>
expect(screen.getAllByRole("menuitem")).toHaveLength(3))).rejects.toThrow();
+ expect(screen.queryByRole("menuitem")).toBeNull();
});
- it("renders a plain button with no menu when Dags is not authorized", () => {
+ it("is not a link when Dags is not authorized", () => {
render(<DagsButton authorizedMenuItems={[]} />, { wrapper: Wrapper });
- expect(getTrigger()).not.toHaveAttribute("aria-haspopup");
+ expect(screen.queryByRole("link", { name: "nav.dags" })).toBeNull();
+ expect(screen.getByRole("button", { name: "nav.dags" })).toBeDisabled();
});
});
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.tsx
index 3f7b7aa5c5d..4c6fbf8db84 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.tsx
@@ -17,59 +17,26 @@
* under the License.
*/
import { useTranslation } from "react-i18next";
-import { Link as RouterLink } from "react-router-dom";
import type { MenuItem } from "openapi/requests/types.gen";
-import { Menu } from "src/system-components";
-
import { DagIcon } from "src/assets/DagIcon";
import { NavButton } from "./NavButton";
-import { NavMenu } from "./NavMenu";
-
-const links = [
- {
- href: "/dags",
- key: "dag_other",
- },
- {
- href: "/dag_runs",
- key: "dagRun_other",
- },
- {
- href: "/task_instances",
- key: "taskInstance_other",
- },
-];
export const DagsButton = ({ authorizedMenuItems }: { readonly
authorizedMenuItems: Array<MenuItem> }) => {
const { t: translate } = useTranslation();
const isAuthorized = authorizedMenuItems.includes("Dags");
- const trigger = (
+ // Without a destination NavButton renders a real <button>, which can be
disabled. A disabled
+ // anchor would stay clickable.
+ return (
<NavButton
data-testid="nav-dags-link"
disabled={!isAuthorized}
icon={DagIcon}
title={translate("nav.dags")}
- to={links.map(({ href }) => href)}
+ to={isAuthorized ? "/dags" : undefined}
/>
);
-
- if (!isAuthorized) {
- return trigger;
- }
-
- return (
- <NavMenu trigger={trigger}>
- {links.map(({ href, key }) => (
- <Menu.Item asChild key={key} value={key}>
- <RouterLink aria-label={translate(key)} to={href}>
- {translate(key)}
- </RouterLink>
- </Menu.Item>
- ))}
- </NavMenu>
- );
};
