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 f3edf4d4c5c Allow multiple routes to show active NavButtons (#69884)
f3edf4d4c5c is described below
commit f3edf4d4c5c2784224acc45b91c259289c0becaf
Author: Brent Bovenzi <[email protected]>
AuthorDate: Tue Jul 21 13:53:24 2026 -0400
Allow multiple routes to show active NavButtons (#69884)
* Allow multiple routes to show active NavButtons
* clean up securitybutton
---
.../src/airflow/ui/src/layouts/Nav/AdminButton.tsx | 23 +++--
.../airflow/ui/src/layouts/Nav/BrowseButton.tsx | 23 +++--
.../src/airflow/ui/src/layouts/Nav/Nav.tsx | 1 +
.../airflow/ui/src/layouts/Nav/NavButton.test.tsx | 114 +++++++++++++++++++++
.../src/airflow/ui/src/layouts/Nav/NavButton.tsx | 37 ++++---
.../src/airflow/ui/src/layouts/Nav/PluginMenus.tsx | 7 +-
.../airflow/ui/src/layouts/Nav/SecurityButton.tsx | 30 +++---
7 files changed, 189 insertions(+), 46 deletions(-)
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/AdminButton.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/AdminButton.tsx
index 427d994c55a..7037b3aaf1e 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/AdminButton.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/AdminButton.tsx
@@ -62,15 +62,14 @@ export const AdminButton = ({
readonly externalViews: Array<NavItemResponse>;
}) => {
const { t: translate } = useTranslation("common");
- const menuItems = links
- .filter(({ title }) => authorizedMenuItems.includes(title as MenuItem))
- .map((link) => (
- <Menu.Item asChild key={link.title} value={link.title}>
- <RouterLink aria-label={translate(`admin.${link.title}`)}
to={link.href}>
- {translate(`admin.${link.title}`)}
- </RouterLink>
- </Menu.Item>
- ));
+ const authorizedLinks = links.filter(({ title }) =>
authorizedMenuItems.includes(title as MenuItem));
+ const menuItems = authorizedLinks.map((link) => (
+ <Menu.Item asChild key={link.title} value={link.title}>
+ <RouterLink aria-label={translate(`admin.${link.title}`)} to={link.href}>
+ {translate(`admin.${link.title}`)}
+ </RouterLink>
+ </Menu.Item>
+ ));
if (!menuItems.length && !externalViews.length) {
return undefined;
@@ -79,7 +78,11 @@ export const AdminButton = ({
return (
<Menu.Root positioning={{ placement: "right" }}>
<Menu.Trigger asChild>
- <NavButton icon={FiSettings} title={translate("nav.admin")} />
+ <NavButton
+ icon={FiSettings}
+ title={translate("nav.admin")}
+ to={authorizedLinks.map(({ href }) => href)}
+ />
</Menu.Trigger>
<Menu.Content>
{menuItems}
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 d95f9e0f318..9ebb236ab07 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx
@@ -63,15 +63,14 @@ export const BrowseButton = ({
readonly externalViews: Array<NavItemResponse>;
}) => {
const { t: translate } = useTranslation("common");
- const menuItems = links
- .filter(({ title }) => authorizedMenuItems.includes(title as MenuItem))
- .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 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>
+ ));
if (!menuItems.length && !externalViews.length) {
return undefined;
@@ -80,7 +79,11 @@ export const BrowseButton = ({
return (
<Menu.Root positioning={{ placement: "right" }}>
<Menu.Trigger asChild>
- <NavButton icon={FiGlobe} title={translate("nav.browse")} />
+ <NavButton
+ icon={FiGlobe}
+ title={translate("nav.browse")}
+ to={authorizedLinks.map(({ href }) => href)}
+ />
</Menu.Trigger>
<Menu.Content>
{menuItems}
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx
index 4e4e9e39b17..39924dc18d0 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx
@@ -176,6 +176,7 @@ export const Nav = () => {
data-testid="nav-dags-link"
disabled={!authLinks?.authorized_menu_items.includes("Dags")}
icon={DagIcon}
+ matchPaths={["dag_runs", "task_instances"]}
title={translate("nav.dags")}
to="dags"
/>
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
new file mode 100644
index 00000000000..6671f4a109a
--- /dev/null
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.test.tsx
@@ -0,0 +1,114 @@
+/*!
+ * 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";
+import { render, screen } from "@testing-library/react";
+import type { PropsWithChildren } from "react";
+import { FiHome } from "react-icons/fi";
+import { MemoryRouter } from "react-router-dom";
+import { describe, expect, it } from "vitest";
+
+import { BaseWrapper } from "src/utils/Wrapper";
+
+import { NavButton } from "./NavButton";
+
+const wrapperAt = (path: string) => {
+ const wrapper = ({ children }: PropsWithChildren) => (
+ <BaseWrapper>
+ <MemoryRouter initialEntries={[path]}>{children}</MemoryRouter>
+ </BaseWrapper>
+ );
+
+ return wrapper;
+};
+
+describe("NavButton", () => {
+ describe("single `to`", () => {
+ it("renders as a link to that destination", () => {
+ render(<NavButton icon={FiHome} title="Dags" to="dags" />, { wrapper:
wrapperAt("/") });
+
+ expect(screen.getByRole("link", { name: "Dags"
})).toHaveAttribute("href", "/dags");
+ });
+
+ it("is active when the current route matches", () => {
+ render(<NavButton icon={FiHome} title="Dags" to="dags" />, { wrapper:
wrapperAt("/dags") });
+
+ expect(screen.getByRole("link", { name: "Dags"
})).toHaveAttribute("aria-current", "page");
+ });
+
+ it("is active on a nested route under the destination", () => {
+ render(<NavButton icon={FiHome} title="Dags" to="dags" />, {
+ wrapper: wrapperAt("/dags/my_dag/runs"),
+ });
+
+ expect(screen.getByRole("link", { name: "Dags"
})).toHaveAttribute("aria-current", "page");
+ });
+
+ it("is not active on an unrelated route", () => {
+ render(<NavButton icon={FiHome} title="Dags" to="dags" />, { wrapper:
wrapperAt("/assets") });
+
+ expect(screen.getByRole("link", { name: "Dags"
})).not.toHaveAttribute("aria-current");
+ });
+ });
+
+ describe("multiple `to`", () => {
+ it("renders as a plain button, not a link", () => {
+ render(<NavButton icon={FiHome} title="Browse" to={["events", "jobs"]}
/>, {
+ wrapper: wrapperAt("/"),
+ });
+
+ const button = screen.getByRole("button", { name: "Browse" });
+
+ expect(button).not.toHaveAttribute("href");
+ });
+
+ it("is active when the current route matches any of the destinations", ()
=> {
+ render(<NavButton icon={FiHome} title="Browse" to={["events", "jobs"]}
/>, {
+ wrapper: wrapperAt("/jobs"),
+ });
+
+ expect(screen.getByRole("button", { name: "Browse"
})).toHaveAttribute("aria-current", "page");
+ });
+
+ it("is not active when the current route matches none of the
destinations", () => {
+ render(<NavButton icon={FiHome} title="Browse" to={["events", "jobs"]}
/>, {
+ wrapper: wrapperAt("/xcoms"),
+ });
+
+ expect(screen.getByRole("button", { name: "Browse"
})).not.toHaveAttribute("aria-current");
+ });
+ });
+
+ 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 f943efc32aa..648ee43ce62 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx
@@ -19,7 +19,9 @@
import { Box, type BoxProps, Button, Icon, type IconProps, Link, type
ButtonProps } from "@chakra-ui/react";
import type { ReactNode, ForwardRefExoticComponent, RefAttributes } from
"react";
import type { IconType } from "react-icons";
-import { Link as RouterLink, useMatch } from "react-router-dom";
+import { Link as RouterLink, matchPath, useLocation } from "react-router-dom";
+
+const noMatchPaths: Array<string> = [];
const commonLabelProps: BoxProps = {
fontSize: "2xs",
@@ -33,21 +35,29 @@ 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;
- readonly to?: string;
+ // A single destination renders the button as a link; an array only affects
isActive matching
+ // (used for buttons like menu triggers that should highlight for any of
several routes).
+ readonly to?: Array<string> | string;
} & ButtonProps;
-export const NavButton = ({ icon, isExternal = false, pluginIcon, title, to,
...rest }: NavButtonProps) => {
- // Use useMatch to determine if the current route matches the button's
destination
- // This provides the same functionality as NavLink's isActive prop
- // Only applies to buttons with a to prop (but needs to be before any return
statements)
- const match = useMatch({
- end: to === "/", // Only exact match for root path
- path: to ?? "",
- });
- // Only applies to buttons with a to prop
- const isActive = Boolean(to) ? Boolean(match) : false;
+export const NavButton = ({
+ icon,
+ isExternal = false,
+ matchPaths = noMatchPaths,
+ pluginIcon,
+ title,
+ to,
+ ...rest
+}: NavButtonProps) => {
+ const { pathname } = useLocation();
+
+ const activePaths = [...(to === undefined ? [] : Array.isArray(to) ? to :
[to]), ...matchPaths];
+ const isActive = activePaths.some((path) => matchPath({ end: path === "/",
path }, pathname) !== null);
const commonButtonProps: ButtonProps = {
_expanded: isActive
@@ -72,6 +82,7 @@ export const NavButton = ({ icon, isExternal = false,
pluginIcon, title, to, ...
color: "fg",
},
alignItems: "center",
+ "aria-current": isActive ? "page" : undefined,
"aria-label": title,
bg: isActive ? "brand.solid" : undefined,
borderRadius: "md",
@@ -92,7 +103,7 @@ export const NavButton = ({ icon, isExternal = false,
pluginIcon, title, to, ...
...rest,
};
- if (to === undefined) {
+ if (to === undefined || Array.isArray(to)) {
return (
<Button {...commonButtonProps}>
{pluginIcon ?? <Icon as={icon} boxSize={5} />}
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/PluginMenus.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/PluginMenus.tsx
index a7cacf8571d..63146715433 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/PluginMenus.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/PluginMenus.tsx
@@ -53,6 +53,11 @@ export const PluginMenus = ({ navItems }: { readonly
navItems: Array<NavItemResp
// A single remaining item is promoted to the toolbar to avoid a one-item
submenu.
const showRemainingInMenu = remainingItems.length >= 2;
+ // Only internal routes (backed by url_route) can match the current
pathname; external views never do.
+ const remainingPaths = remainingItems
+ .filter((navItem) => navItem.url_route !== undefined && navItem.url_route
!== null)
+ .map((navItem) => `plugin/${navItem.url_route}`);
+
return (
<>
{promotedItems.map((navItem) => (
@@ -61,7 +66,7 @@ export const PluginMenus = ({ navItems }: { readonly
navItems: Array<NavItemResp
{showRemainingInMenu ? (
<Menu.Root positioning={{ placement: "right" }}>
<Menu.Trigger>
- <NavButton as={Box} icon={LuPlug} title={translate("nav.plugins")}
/>
+ <NavButton as={Box} icon={LuPlug} title={translate("nav.plugins")}
to={remainingPaths} />
</Menu.Trigger>
<Menu.Content>
{remainingButtons.map((navItem) => (
diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/SecurityButton.tsx
b/airflow-core/src/airflow/ui/src/layouts/Nav/SecurityButton.tsx
index 2955abce5ee..ccfa34b5af5 100644
--- a/airflow-core/src/airflow/ui/src/layouts/Nav/SecurityButton.tsx
+++ b/airflow-core/src/airflow/ui/src/layouts/Nav/SecurityButton.tsx
@@ -33,23 +33,29 @@ export const SecurityButton = () => {
return undefined;
}
+ const securityItems = authLinks.extra_menu_items.map(({ text }) => {
+ const securityKey = text.toLowerCase().replace(" ", "-");
+
+ return { path: `security/${securityKey}`, securityKey, text };
+ });
+
return (
<Menu.Root positioning={{ placement: "right" }}>
<Menu.Trigger asChild>
- <NavButton icon={FiLock} title={translate("nav.security")} />
+ <NavButton
+ icon={FiLock}
+ title={translate("nav.security")}
+ to={securityItems.map(({ path }) => path)}
+ />
</Menu.Trigger>
<Menu.Content>
- {authLinks.extra_menu_items.map(({ text }) => {
- const securityKey = text.toLowerCase().replace(" ", "-");
-
- return (
- <Menu.Item asChild key={text} value={text}>
- <Link aria-label={text} to={`security/${securityKey}`}>
- {translate(`security.${securityKey}`)}
- </Link>
- </Menu.Item>
- );
- })}
+ {securityItems.map(({ path, securityKey, text }) => (
+ <Menu.Item asChild key={text} value={text}>
+ <Link aria-label={text} to={path}>
+ {translate(`security.${securityKey}`)}
+ </Link>
+ </Menu.Item>
+ ))}
</Menu.Content>
</Menu.Root>
);
