This is an automated email from the ASF dual-hosted git repository. ryanahamilton pushed a commit to branch backport-nav-updates-to-v3-1-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 0eae7b3eba49e13c96c798ed551ae9ae99c588fa Author: Ryan Hamilton <[email protected]> AuthorDate: Wed Oct 29 07:09:24 2025 -0400 Refine the visual design, interaction, and accessibility of the global navigation (#57455) --- .../src/airflow/ui/src/layouts/Nav/AdminButton.tsx | 2 +- .../airflow/ui/src/layouts/Nav/BrowseButton.tsx | 2 +- .../src/airflow/ui/src/layouts/Nav/DocsButton.tsx | 13 +- .../src/airflow/ui/src/layouts/Nav/Nav.tsx | 25 ++-- .../src/airflow/ui/src/layouts/Nav/NavButton.tsx | 140 +++++++++++++------- .../src/airflow/ui/src/layouts/Nav/PluginMenus.tsx | 2 +- .../airflow/ui/src/layouts/Nav/SecurityButton.tsx | 2 +- .../ui/src/layouts/Nav/TimezoneMenuItem.tsx | 5 +- .../ui/src/layouts/Nav/UserSettingsButton.tsx | 143 ++++++++++----------- 9 files changed, 188 insertions(+), 146 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 ddc01d9db28..427d994c55a 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/AdminButton.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/AdminButton.tsx @@ -79,7 +79,7 @@ export const AdminButton = ({ return ( <Menu.Root positioning={{ placement: "right" }}> <Menu.Trigger asChild> - <NavButton icon={<FiSettings size={28} />} title={translate("nav.admin")} /> + <NavButton icon={FiSettings} title={translate("nav.admin")} /> </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 170f89a94f5..986fa973bbc 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx @@ -70,7 +70,7 @@ export const BrowseButton = ({ return ( <Menu.Root positioning={{ placement: "right" }}> <Menu.Trigger asChild> - <NavButton icon={<FiGlobe size={28} />} title={translate("nav.browse")} /> + <NavButton icon={FiGlobe} title={translate("nav.browse")} /> </Menu.Trigger> <Menu.Content> {menuItems} diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/DocsButton.tsx b/airflow-core/src/airflow/ui/src/layouts/Nav/DocsButton.tsx index d73b323b3e1..3af4e7f93a6 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/DocsButton.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/DocsButton.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { Link } from "@chakra-ui/react"; +import { Box, Icon, Link } from "@chakra-ui/react"; import { useTranslation } from "react-i18next"; import { FiBookOpen, FiExternalLink } from "react-icons/fi"; @@ -61,7 +61,7 @@ export const DocsButton = ({ return ( <Menu.Root positioning={{ placement: "right" }}> <Menu.Trigger asChild> - <NavButton icon={<FiBookOpen size={28} />} title={translate("nav.docs")} /> + <NavButton icon={FiBookOpen} title={translate("nav.docs")} /> </Menu.Trigger> <Menu.Content> {links @@ -73,17 +73,18 @@ export const DocsButton = ({ href={link.href} rel="noopener noreferrer" target="_blank" + textDecoration="none" > - {translate(`docs.${link.key}`)} - <FiExternalLink /> + <Box flex="1">{translate(`docs.${link.key}`)}</Box> + <Icon as={FiExternalLink} boxSize={4} color="fg.muted" /> </Link> </Menu.Item> ))} {version === undefined ? undefined : ( <Menu.Item asChild key={version} value={version}> <Link aria-label={version} href={versionLink} rel="noopener noreferrer" target="_blank"> - {version} - <FiExternalLink /> + <Box flex="1">{version}</Box> + <Icon as={FiExternalLink} boxSize={4} color="fg.muted" /> </Link> </Menu.Item> )} 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 01185d2df8b..5fa459b7959 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx @@ -19,7 +19,7 @@ import { Box, Flex, VStack } from "@chakra-ui/react"; import { useTranslation } from "react-i18next"; import { FiDatabase, FiHome } from "react-icons/fi"; -import { NavLink } from "react-router-dom"; +import { Link } from "react-router-dom"; import { useAuthLinksServiceGetAuthMenus, @@ -140,14 +140,14 @@ export const Nav = () => { height="100%" justifyContent="space-between" position="fixed" - py={3} + py={1} top={0} - width={20} + width={16} zIndex="docked" > - <Flex alignItems="center" flexDir="column" width="100%"> - <Box mb={3}> - <NavLink to="/"> + <Flex alignItems="center" flexDir="column" width="100%" gap={1}> + <Box asChild boxSize={14} display="flex" alignItems="center" justifyContent="center"> + <Link to="/" title={translate("nav.home")}> <AirflowPin _motionSafe={{ _hover: { @@ -155,21 +155,20 @@ export const Nav = () => { transition: "transform 0.8s ease-in-out", }, }} - height="35px" - width="35px" + boxSize={8} /> - </NavLink> + </Link> </Box> - <NavButton icon={<FiHome size="28px" />} title={translate("nav.home")} to="/" /> + <NavButton icon={FiHome} title={translate("nav.home")} to="/" /> <NavButton disabled={!authLinks?.authorized_menu_items.includes("Dags")} - icon={<DagIcon height="28px" width="28px" />} + icon={DagIcon} title={translate("nav.dags")} to="dags" /> <NavButton disabled={!authLinks?.authorized_menu_items.includes("Assets")} - icon={<FiDatabase size="28px" />} + icon={FiDatabase} title={translate("nav.assets")} to="assets" /> @@ -184,7 +183,7 @@ export const Nav = () => { <SecurityButton /> <PluginMenus navItems={navItemsWithLegacy} /> </Flex> - <Flex flexDir="column"> + <Flex flexDir="column" gap={1}> <DocsButton externalViews={docsItems} showAPI={authLinks?.authorized_menu_items.includes("Docs")} 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 e8f14803366..3ebea71bef1 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx @@ -16,62 +16,104 @@ * specific language governing permissions and limitations * under the License. */ -import { Box, Button, Link, type ButtonProps } from "@chakra-ui/react"; -import type { ReactElement } from "react"; -import { NavLink } from "react-router-dom"; +import { Box, type BoxProps, Button, Icon, type IconProps, Link, type ButtonProps } from "@chakra-ui/react"; +import { useMemo, type ForwardRefExoticComponent, type RefAttributes } from "react"; +import { IconType } from "react-icons"; +import { Link as RouterLink, useMatch } from "react-router-dom"; -const styles = { - _active: { - bg: "brand.emphasized", - }, - // Fix inverted hover and active colors - _hover: { - bg: "brand.emphasized", // Even darker for better light mode contrast - }, - alignItems: "center", - borderRadius: "none", - colorPalette: "brand", - flexDir: "column", - height: 20, - variant: "ghost", - whiteSpace: "wrap", - width: 20, -} satisfies ButtonProps; +const commonLabelProps: BoxProps = { + fontSize: "2xs", + overflow: "hidden", + textAlign: "center", + textOverflow: "ellipsis", + whiteSpace: "nowrap", + width: "full", +}; type NavButtonProps = { - readonly icon: ReactElement; + readonly icon: IconType | ForwardRefExoticComponent<IconProps & RefAttributes<SVGSVGElement>>; readonly isExternal?: boolean; - readonly title?: string; + readonly title: string; readonly to?: string; } & ButtonProps; -export const NavButton = ({ icon, isExternal = false, title, to, ...rest }: NavButtonProps) => - to === undefined ? ( - <Button {...styles} {...rest}> - <Box alignSelf="center">{icon}</Box> - <Box fontSize="xs">{title}</Box> - </Button> - ) : isExternal ? ( - <Link href={to} px={2} rel="noopener noreferrer" target="_blank"> - <Button {...styles} variant="ghost" {...rest}> - <Box alignSelf="center">{icon}</Box> - <Box fontSize="xs">{title}</Box> +export const NavButton = ({ icon, isExternal = false, 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 = to ? useMatch({ + path: to, + end: to === "/" // Only exact match for root path + }) : undefined; + // Only applies to buttons with a to prop + const isActive = Boolean(match); + + const commonButtonProps = useMemo<ButtonProps>(() => ({ + _expanded: isActive ? undefined : { + bg: "brand.emphasized", // Even darker for better light mode contrast + color: "fg", + }, + _focus: isActive ? undefined : { + color: "fg", + }, + _hover: isActive ? undefined : { + bg: "brand.emphasized", // Even darker for better light mode contrast + color: "fg", + _active: { + bg: "brand.solid", + color: "white", + }, + }, + alignItems: "center", + bg: isActive ? "brand.solid" : undefined, + borderRadius: "md", + borderWidth: 0, + boxSize: 14, + color: isActive ? "white" : "fg.muted", + colorPalette: "brand", + cursor: "pointer", + flexDir: "column", + gap: 0, + overflow: "hidden", + padding: 0, + textDecoration: "none", + title, + transition: "background-color 0.2s ease, color 0.2s ease", + variant: "plain", + whiteSpace: "wrap", + ...rest, + }), [isActive, rest, title]); + + if (to === undefined) { + return ( + <Button {...commonButtonProps}> + <Icon as={icon} boxSize={5} /> + <Box {...commonLabelProps}>{title}</Box> </Button> - </Link> - ) : ( - <NavLink to={to}> - {({ isActive }: { readonly isActive: boolean }) => ( - <Button - {...styles} - _active={isActive ? { bg: "brand.solid" } : { bg: "brand.emphasized" }} - // Override styles for active state to ensure proper colors - _hover={isActive ? { bg: "brand.solid" } : { bg: "brand.emphasized" }} - variant={isActive ? "solid" : "ghost"} - {...rest} - > - <Box alignSelf="center">{icon}</Box> - <Box fontSize="xs">{title}</Box> + ); + } + + if (isExternal) { + return ( + <Link href={to} asChild rel="noopener noreferrer" target="_blank"> + <Button {...commonButtonProps}> + <Icon as={icon} boxSize={5} /> + <Box {...commonLabelProps}>{title}</Box> </Button> - )} - </NavLink> + </Link> + ); + } + + return ( + <Button + as={Link} + asChild + {...commonButtonProps} + > + <RouterLink to={to}> + <Icon as={icon} boxSize={5} /> + <Box {...commonLabelProps}>{title}</Box> + </RouterLink> + </Button> ); +}; 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 2dfbf58ac3d..c8727804890 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/PluginMenus.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/PluginMenus.tsx @@ -53,7 +53,7 @@ export const PluginMenus = ({ navItems }: { readonly navItems: Array<NavItemResp return navItems.length >= 2 ? ( <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")} /> </Menu.Trigger> <Menu.Content> {buttons.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 6d9fdca68f0..2955abce5ee 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/SecurityButton.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/SecurityButton.tsx @@ -36,7 +36,7 @@ export const SecurityButton = () => { return ( <Menu.Root positioning={{ placement: "right" }}> <Menu.Trigger asChild> - <NavButton icon={<FiLock size={28} />} title={translate("nav.security")} /> + <NavButton icon={FiLock} title={translate("nav.security")} /> </Menu.Trigger> <Menu.Content> {authLinks.extra_menu_items.map(({ text }) => { diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/TimezoneMenuItem.tsx b/airflow-core/src/airflow/ui/src/layouts/Nav/TimezoneMenuItem.tsx index 596cf79bb1b..cb13b3c9763 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/TimezoneMenuItem.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/TimezoneMenuItem.tsx @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { Box, Icon } from "@chakra-ui/react"; import dayjs from "dayjs"; import timezone from "dayjs/plugin/timezone"; import utc from "dayjs/plugin/utc"; @@ -48,8 +49,8 @@ export const TimezoneMenuItem = ({ onOpen }: { readonly onOpen: () => void }) => return ( <Menu.Item onClick={onOpen} value="timezone"> - <FiClock size={20} style={{ marginRight: "8px" }} /> - {translate("timezone")}: {dayjs(time).tz(selectedTimezone).format("HH:mm z (Z)")} + <Icon as={FiClock} boxSize={4} /> + <Box flex="1">{translate("timezone")}: {dayjs(time).tz(selectedTimezone).format("HH:mm z (Z)")}</Box> </Menu.Item> ); }; diff --git a/airflow-core/src/airflow/ui/src/layouts/Nav/UserSettingsButton.tsx b/airflow-core/src/airflow/ui/src/layouts/Nav/UserSettingsButton.tsx index 31e2dd0078a..15ee91b0422 100644 --- a/airflow-core/src/airflow/ui/src/layouts/Nav/UserSettingsButton.tsx +++ b/airflow-core/src/airflow/ui/src/layouts/Nav/UserSettingsButton.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useDisclosure } from "@chakra-ui/react"; +import { Box, Icon, useDisclosure } from "@chakra-ui/react"; import { useTranslation } from "react-i18next"; import { FiGrid, @@ -55,9 +55,29 @@ type ColorMode = (typeof COLOR_MODES)[keyof typeof COLOR_MODES]; export const UserSettingsButton = ({ externalViews }: { readonly externalViews: Array<NavItemResponse> }) => { const { i18n, t: translate } = useTranslation(); const { selectedTheme, setColorMode } = useColorMode(); + + const colorModeOptions = [ + { + icon: FiSun, + label: translate("appearance.lightMode"), + value: COLOR_MODES.LIGHT, + }, + { + icon: FiMoon, + label: translate("appearance.darkMode"), + value: COLOR_MODES.DARK, + }, + { + icon: FiMonitor, + label: translate("appearance.systemMode"), + value: COLOR_MODES.SYSTEM, + }, + ]; + const { onClose: onCloseTimezone, onOpen: onOpenTimezone, open: isOpenTimezone } = useDisclosure(); const { onClose: onCloseLogout, onOpen: onOpenLogout, open: isOpenLogout } = useDisclosure(); const { onClose: onCloseLanguage, onOpen: onOpenLanguage, open: isOpenLanguage } = useDisclosure(); + const [dagView, setDagView] = useLocalStorage<"graph" | "grid">("default_dag_view", "grid"); const theme = selectedTheme ?? COLOR_MODES.SYSTEM; @@ -65,79 +85,58 @@ export const UserSettingsButton = ({ externalViews }: { readonly externalViews: const isRTL = i18n.dir() === "rtl"; return ( - <Menu.Root positioning={{ placement: "right" }}> - <Menu.Trigger asChild> - <NavButton icon={<FiUser size={28} />} title={translate("user")} /> - </Menu.Trigger> - <Menu.Content> - <Menu.Item onClick={onOpenLanguage} value="language"> - <FiGlobe size={20} style={{ marginRight: "8px" }} /> - {translate("selectLanguage")} - </Menu.Item> - <Menu.Root> - <Menu.TriggerItem> - <FiEye size={20} style={{ marginRight: "8px" }} /> - {translate("appearance.appearance")} - {isRTL ? ( - <FiChevronLeft size={20} style={{ marginRight: "auto" }} /> - ) : ( - <FiChevronRight size={20} style={{ marginLeft: "auto" }} /> - )} - </Menu.TriggerItem> - <Menu.Content> - <Menu.RadioItemGroup - onValueChange={(element) => setColorMode(element.value as ColorMode)} - value={theme} - > - <Menu.RadioItem value={COLOR_MODES.LIGHT}> - <FiSun size={20} style={{ marginRight: "8px" }} /> - {translate("appearance.lightMode")} - <Menu.ItemIndicator /> - </Menu.RadioItem> - <Menu.RadioItem value={COLOR_MODES.DARK}> - <FiMoon size={20} style={{ marginRight: "8px" }} /> - {translate("appearance.darkMode")} - <Menu.ItemIndicator /> - </Menu.RadioItem> - <Menu.RadioItem value={COLOR_MODES.SYSTEM}> - <FiMonitor size={20} style={{ marginRight: "8px" }} /> - {translate("appearance.systemMode")} - <Menu.ItemIndicator /> - </Menu.RadioItem> - </Menu.RadioItemGroup> - </Menu.Content> - </Menu.Root> - <Menu.Item - onClick={() => (dagView === "grid" ? setDagView("graph") : setDagView("grid"))} - value={dagView} - > - {dagView === "grid" ? ( - <> - <MdOutlineAccountTree size={20} style={{ marginRight: "8px" }} /> - {translate("defaultToGraphView")} - </> - ) : ( - <> - <FiGrid size={20} style={{ marginRight: "8px" }} /> - {translate("defaultToGridView")} - </> - )} - </Menu.Item> - <TimezoneMenuItem onOpen={onOpenTimezone} /> - {externalViews.map((view) => ( - <PluginMenuItem {...view} key={view.name} /> - ))} - <Menu.Item onClick={onOpenLogout} value="logout"> - <FiLogOut - size={20} - style={{ marginRight: "8px", transform: isRTL ? "rotate(180deg)" : undefined }} - /> - {translate("logout")} - </Menu.Item> - </Menu.Content> + <> + <Menu.Root positioning={{ placement: "right" }}> + <Menu.Trigger asChild> + <NavButton icon={FiUser} title={translate("user")} /> + </Menu.Trigger> + <Menu.Content> + <Menu.Item onClick={onOpenLanguage} value="language"> + <Icon as={FiGlobe} boxSize={4} /> + <Box flex="1">{translate("selectLanguage")}</Box> + </Menu.Item> + <Menu.Root> + <Menu.TriggerItem> + <Icon as={FiEye} boxSize={4} /> + <Box flex="1">{translate("appearance.appearance")}</Box> + <Icon as={isRTL ? FiChevronLeft : FiChevronRight} boxSize={4} color="fg.muted" /> + </Menu.TriggerItem> + <Menu.Content> + <Menu.RadioItemGroup + onValueChange={(element) => setColorMode(element.value as ColorMode)} + value={theme} + > + {colorModeOptions.map(({ icon, label, value }) => ( + <Menu.RadioItem key={value} value={value}> + <Icon as={icon} boxSize={4} /> + <Box flex="1">{label}</Box> + <Menu.ItemIndicator color="fg.muted" /> + </Menu.RadioItem> + ))} + </Menu.RadioItemGroup> + </Menu.Content> + </Menu.Root> + <Menu.Item + onClick={() => (dagView === "grid" ? setDagView("graph") : setDagView("grid"))} + value={dagView} + > + <Icon as={dagView === "grid" ? MdOutlineAccountTree : FiGrid} boxSize={4} /> + <Box flex="1">{dagView === "grid" ? translate("defaultToGraphView") : translate("defaultToGridView")}</Box> + </Menu.Item> + <TimezoneMenuItem onOpen={onOpenTimezone} /> + {externalViews.map((view) => ( + <PluginMenuItem {...view} key={view.name} /> + ))} + <Menu.Separator /> + <Menu.Item onClick={onOpenLogout} value="logout"> + <Icon as={FiLogOut} boxSize={4} transform={isRTL ? "rotate(180deg)" : undefined} /> + <Box flex="1">{translate("logout")}</Box> + </Menu.Item> + </Menu.Content> + </Menu.Root> <LanguageModal isOpen={isOpenLanguage} onClose={onCloseLanguage} /> <TimezoneModal isOpen={isOpenTimezone} onClose={onCloseTimezone} /> <LogoutModal isOpen={isOpenLogout} onClose={onCloseLogout} /> - </Menu.Root> + </> ); };
