This is an automated email from the ASF dual-hosted git repository.
bbovenzi pushed a commit to branch v3-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-4-test by this push:
new 32c12f2bebd [v3-4-test] Open the Dags menu from the Dags nav button
instead of navigating (#75092) (#75109)
32c12f2bebd is described below
commit 32c12f2bebdaa2b0692c08f8cae21e26210515a9
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Oct 19 18:04:41 2026 -0400
[v3-4-test] Open the Dags menu from the Dags nav button instead of
navigating (#75092) (#75109)
The Dags button now heads a menu like Browse and Security, but it was
still a link. A click navigated to the Dags list and opened the menu
over the new page at the same time, which reads as a flicker. Treating
it as a menu trigger makes it behave like its siblings: hover or click
opens the menu, and the Dags list is one of its entries.
(cherry picked from commit ab1acec7aa831f77b6c58f76010e5613933c797c)
Co-authored-by: Pierre Jeambrun <[email protected]>
---
.../airflow/ui/src/layouts/Nav/DagsButton.test.tsx | 36 +++++++++++++++++++---
.../src/airflow/ui/src/layouts/Nav/DagsButton.tsx | 4 +--
.../airflow/ui/src/layouts/Nav/NavButton.test.tsx | 18 -----------
.../src/airflow/ui/src/layouts/Nav/NavButton.tsx | 17 ++--------
4 files changed, 35 insertions(+), 40 deletions(-)
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 381f12de0f3..b1146cd4eb4 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
@@ -16,23 +16,51 @@
* 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 { Wrapper } from "src/utils/Wrapper";
+import { BaseWrapper, Wrapper } from "src/utils/Wrapper";
import { DagsButton } from "./DagsButton";
-const getTrigger = () => screen.getByRole("link", { name: "nav.dags" });
+const getTrigger = () => screen.getByRole("button", { name: "nav.dags" });
+
+const wrapperAt = (path: string) => {
+ const wrapper = ({ children }: PropsWithChildren) => (
+ <BaseWrapper>
+ <MemoryRouter initialEntries={[path]}>{children}</MemoryRouter>
+ </BaseWrapper>
+ );
+
+ return wrapper;
+};
describe("DagsButton", () => {
- it("links straight to the Dags list", () => {
+ it("opens the menu on click instead of navigating", async () => {
render(<DagsButton authorizedMenuItems={["Dags"]} />, { wrapper: Wrapper
});
- expect(getTrigger()).toHaveAttribute("href", "/dags");
+ expect(getTrigger()).not.toHaveAttribute("href");
+
+ fireEvent.click(getTrigger());
+
+ await waitFor(() => {
+ expect(screen.getAllByRole("menuitem")).toHaveLength(3);
+ });
});
+ 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
});
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 4618285a92c..3f7b7aa5c5d 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/DagsButton.tsx
@@ -47,15 +47,13 @@ export const DagsButton = ({ authorizedMenuItems }: {
readonly authorizedMenuIte
const { t: translate } = useTranslation();
const isAuthorized = authorizedMenuItems.includes("Dags");
- // Stays a link to the Dags list: hovering reveals the menu, clicking goes
straight there.
const trigger = (
<NavButton
data-testid="nav-dags-link"
disabled={!isAuthorized}
icon={DagIcon}
- matchPaths={["dag_runs", "task_instances"]}
title={translate("nav.dags")}
- to="/dags"
+ to={links.map(({ href }) => href)}
/>
);
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.test.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.test.tsx
index 696441f218a..37b10be834c 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.test.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.test.tsx
@@ -108,22 +108,4 @@ describe("NavButton", () => {
expect(link).toHaveAttribute("rel", "noopener noreferrer");
});
});
-
- describe("matchPaths", () => {
- it("is active on an extra match path even though `to` points elsewhere",
() => {
- render(<NavButton icon={FiHome} matchPaths={["dag_runs",
"task_instances"]} title="Dags" to="dags" />, {
- wrapper: wrapperAt("/dag_runs"),
- });
-
- expect(screen.getByRole("link", { name: "Dags"
})).toHaveAttribute("aria-current", "page");
- });
-
- it("is not active on a route outside both `to` and matchPaths", () => {
- render(<NavButton icon={FiHome} matchPaths={["dag_runs",
"task_instances"]} title="Dags" to="dags" />, {
- wrapper: wrapperAt("/assets"),
- });
-
- expect(screen.getByRole("link", { name: "Dags"
})).not.toHaveAttribute("aria-current");
- });
- });
});
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx
index 8b04f107330..5b1f7d6c2e0 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx
@@ -22,8 +22,6 @@ import { Box, type BoxProps, Button, Icon, type IconProps,
Link, type ButtonProp
import type { IconType } from "react-icons";
import { Link as RouterLink, matchPath, useLocation } from "react-router-dom";
-const noMatchPaths: Array<string> = [];
-
const commonLabelProps: BoxProps = {
fontSize: "2xs",
overflow: "hidden",
@@ -36,9 +34,6 @@ const commonLabelProps: BoxProps = {
type NavButtonProps = {
readonly icon: ForwardRefExoticComponent<IconProps &
RefAttributes<SVGSVGElement>> | IconType;
readonly isExternal?: boolean;
- // Extra routes that should also mark this button active, on top of `to`
(e.g. the Dags button
- // should also highlight for the standalone dag runs and task instances
routes).
- readonly matchPaths?: Array<string>;
readonly pluginIcon?: ReactNode;
readonly title: string;
// A single destination renders the button as a link; an array only affects
isActive matching
@@ -46,18 +41,10 @@ type NavButtonProps = {
readonly to?: Array<string> | string;
} & ButtonProps;
-export const NavButton = ({
- icon,
- isExternal = false,
- matchPaths = noMatchPaths,
- pluginIcon,
- title,
- to,
- ...rest
-}: NavButtonProps) => {
+export const NavButton = ({ icon, isExternal = false, pluginIcon, title, to,
...rest }: NavButtonProps) => {
const { pathname } = useLocation();
- const activePaths = [...(to === undefined ? [] : Array.isArray(to) ? to :
[to]), ...matchPaths];
+ const activePaths = to === undefined ? [] : Array.isArray(to) ? to : [to];
const isActive = activePaths.some((path) => matchPath({ end: path === "/",
path }, pathname) !== null);
const commonButtonProps: ButtonProps = {
