Skip to content

Commit 9c8ae23

Browse files
ryanahamiltonkaxil
andcommitted
Refine the visual design, interaction, and accessibility of the global navigation (#57455)(#57565)
* Refine the visual design, interaction, and accessibility of the global navigation (#57455) * Add settings to auto-apply linting, fix linting errors (#57510) * Fix static checks --------- Co-authored-by: Kaxil Naik <[email protected]>
1 parent 6c3a1d5 commit 9c8ae23

File tree

13 files changed

+254
-158
lines changed

13 files changed

+254
-158
lines changed

.gitignore

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,6 @@ npm-debug.log*
171171
yarn-debug.log*
172172
yarn-error.log*
173173
pnpm-debug.log*
174-
.vscode/*
175-
!.vscode/extensions.json
176174
/.vite/
177175
# Exclude the ui .vite dir
178176
airflow-core/src/airflow/ui/.vite/

.vscode/extensions.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"recommendations": [
3+
"esbenp.prettier-vscode",
4+
]
5+
}

.vscode/settings.json

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"[javascript]": {
3+
"editor.defaultFormatter": "esbenp.prettier-vscode",
4+
"editor.formatOnSave": true,
5+
"editor.codeActionsOnSave": {
6+
"source.fixAll.eslint": "explicit"
7+
}
8+
},
9+
"[javascriptreact]": {
10+
"editor.defaultFormatter": "esbenp.prettier-vscode",
11+
"editor.formatOnSave": true,
12+
"editor.codeActionsOnSave": {
13+
"source.fixAll.eslint": "explicit"
14+
}
15+
},
16+
"[typescript]": {
17+
"editor.defaultFormatter": "esbenp.prettier-vscode",
18+
"editor.formatOnSave": true,
19+
"editor.codeActionsOnSave": {
20+
"source.fixAll.eslint": "explicit"
21+
}
22+
},
23+
"[typescriptreact]": {
24+
"editor.defaultFormatter": "esbenp.prettier-vscode",
25+
"editor.formatOnSave": true,
26+
"editor.codeActionsOnSave": {
27+
"source.fixAll.eslint": "explicit"
28+
}
29+
},
30+
"[json]": {
31+
"editor.defaultFormatter": "esbenp.prettier-vscode",
32+
"editor.formatOnSave": true
33+
},
34+
"[jsonc]": {
35+
"editor.defaultFormatter": "esbenp.prettier-vscode",
36+
"editor.formatOnSave": true
37+
},
38+
}

airflow-core/src/airflow/ui/src/layouts/Nav/AdminButton.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export const AdminButton = ({
7979
return (
8080
<Menu.Root positioning={{ placement: "right" }}>
8181
<Menu.Trigger asChild>
82-
<NavButton icon={<FiSettings size={28} />} title={translate("nav.admin")} />
82+
<NavButton icon={FiSettings} title={translate("nav.admin")} />
8383
</Menu.Trigger>
8484
<Menu.Content>
8585
{menuItems}

airflow-core/src/airflow/ui/src/layouts/Nav/BrowseButton.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export const BrowseButton = ({
7070
return (
7171
<Menu.Root positioning={{ placement: "right" }}>
7272
<Menu.Trigger asChild>
73-
<NavButton icon={<FiGlobe size={28} />} title={translate("nav.browse")} />
73+
<NavButton icon={FiGlobe} title={translate("nav.browse")} />
7474
</Menu.Trigger>
7575
<Menu.Content>
7676
{menuItems}

airflow-core/src/airflow/ui/src/layouts/Nav/DocsButton.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
import { Link } from "@chakra-ui/react";
19+
import { Box, Icon, Link } from "@chakra-ui/react";
2020
import { useTranslation } from "react-i18next";
2121
import { FiBookOpen, FiExternalLink } from "react-icons/fi";
2222

@@ -61,7 +61,7 @@ export const DocsButton = ({
6161
return (
6262
<Menu.Root positioning={{ placement: "right" }}>
6363
<Menu.Trigger asChild>
64-
<NavButton icon={<FiBookOpen size={28} />} title={translate("nav.docs")} />
64+
<NavButton icon={FiBookOpen} title={translate("nav.docs")} />
6565
</Menu.Trigger>
6666
<Menu.Content>
6767
{links
@@ -73,17 +73,18 @@ export const DocsButton = ({
7373
href={link.href}
7474
rel="noopener noreferrer"
7575
target="_blank"
76+
textDecoration="none"
7677
>
77-
{translate(`docs.${link.key}`)}
78-
<FiExternalLink />
78+
<Box flex="1">{translate(`docs.${link.key}`)}</Box>
79+
<Icon as={FiExternalLink} boxSize={4} color="fg.muted" />
7980
</Link>
8081
</Menu.Item>
8182
))}
8283
{version === undefined ? undefined : (
8384
<Menu.Item asChild key={version} value={version}>
8485
<Link aria-label={version} href={versionLink} rel="noopener noreferrer" target="_blank">
85-
{version}
86-
<FiExternalLink />
86+
<Box flex="1">{version}</Box>
87+
<Icon as={FiExternalLink} boxSize={4} color="fg.muted" />
8788
</Link>
8889
</Menu.Item>
8990
)}

airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import { Box, Flex, VStack } from "@chakra-ui/react";
2020
import { useTranslation } from "react-i18next";
2121
import { FiDatabase, FiHome } from "react-icons/fi";
22-
import { NavLink } from "react-router-dom";
22+
import { Link } from "react-router-dom";
2323

2424
import {
2525
useAuthLinksServiceGetAuthMenus,
@@ -140,36 +140,35 @@ export const Nav = () => {
140140
height="100%"
141141
justifyContent="space-between"
142142
position="fixed"
143-
py={3}
143+
py={1}
144144
top={0}
145-
width={20}
145+
width={16}
146146
zIndex="docked"
147147
>
148-
<Flex alignItems="center" flexDir="column" width="100%">
149-
<Box mb={3}>
150-
<NavLink to="/">
148+
<Flex alignItems="center" flexDir="column" gap={1} width="100%">
149+
<Box alignItems="center" asChild boxSize={14} display="flex" justifyContent="center">
150+
<Link title={translate("nav.home")} to="/">
151151
<AirflowPin
152152
_motionSafe={{
153153
_hover: {
154154
transform: "rotate(360deg)",
155155
transition: "transform 0.8s ease-in-out",
156156
},
157157
}}
158-
height="35px"
159-
width="35px"
158+
boxSize={8}
160159
/>
161-
</NavLink>
160+
</Link>
162161
</Box>
163-
<NavButton icon={<FiHome size="28px" />} title={translate("nav.home")} to="/" />
162+
<NavButton icon={FiHome} title={translate("nav.home")} to="/" />
164163
<NavButton
165164
disabled={!authLinks?.authorized_menu_items.includes("Dags")}
166-
icon={<DagIcon height="28px" width="28px" />}
165+
icon={DagIcon}
167166
title={translate("nav.dags")}
168167
to="dags"
169168
/>
170169
<NavButton
171170
disabled={!authLinks?.authorized_menu_items.includes("Assets")}
172-
icon={<FiDatabase size="28px" />}
171+
icon={FiDatabase}
173172
title={translate("nav.assets")}
174173
to="assets"
175174
/>
@@ -184,7 +183,7 @@ export const Nav = () => {
184183
<SecurityButton />
185184
<PluginMenus navItems={navItemsWithLegacy} />
186185
</Flex>
187-
<Flex flexDir="column">
186+
<Flex flexDir="column" gap={1}>
188187
<DocsButton
189188
externalViews={docsItems}
190189
showAPI={authLinks?.authorized_menu_items.includes("Docs")}

airflow-core/src/airflow/ui/src/layouts/Nav/NavButton.tsx

Lines changed: 97 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -16,62 +16,110 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
import { Box, Button, Link, type ButtonProps } from "@chakra-ui/react";
20-
import type { ReactElement } from "react";
21-
import { NavLink } from "react-router-dom";
19+
import { Box, type BoxProps, Button, Icon, type IconProps, Link, type ButtonProps } from "@chakra-ui/react";
20+
import { type ReactNode, useMemo, type ForwardRefExoticComponent, type RefAttributes } from "react";
21+
import type { IconType } from "react-icons";
22+
import { Link as RouterLink, useMatch } from "react-router-dom";
2223

23-
const styles = {
24-
_active: {
25-
bg: "brand.emphasized",
26-
},
27-
// Fix inverted hover and active colors
28-
_hover: {
29-
bg: "brand.emphasized", // Even darker for better light mode contrast
30-
},
31-
alignItems: "center",
32-
borderRadius: "none",
33-
colorPalette: "brand",
34-
flexDir: "column",
35-
height: 20,
36-
variant: "ghost",
37-
whiteSpace: "wrap",
38-
width: 20,
39-
} satisfies ButtonProps;
24+
const commonLabelProps: BoxProps = {
25+
fontSize: "2xs",
26+
overflow: "hidden",
27+
textAlign: "center",
28+
textOverflow: "ellipsis",
29+
whiteSpace: "nowrap",
30+
width: "full",
31+
};
4032

4133
type NavButtonProps = {
42-
readonly icon: ReactElement;
34+
readonly icon: ForwardRefExoticComponent<IconProps & RefAttributes<SVGSVGElement>> | IconType;
4335
readonly isExternal?: boolean;
44-
readonly title?: string;
36+
readonly pluginIcon?: ReactNode;
37+
readonly title: string;
4538
readonly to?: string;
4639
} & ButtonProps;
4740

48-
export const NavButton = ({ icon, isExternal = false, title, to, ...rest }: NavButtonProps) =>
49-
to === undefined ? (
50-
<Button {...styles} {...rest}>
51-
<Box alignSelf="center">{icon}</Box>
52-
<Box fontSize="xs">{title}</Box>
53-
</Button>
54-
) : isExternal ? (
55-
<Link href={to} px={2} rel="noopener noreferrer" target="_blank">
56-
<Button {...styles} variant="ghost" {...rest}>
57-
<Box alignSelf="center">{icon}</Box>
58-
<Box fontSize="xs">{title}</Box>
41+
export const NavButton = ({ icon, isExternal = false, pluginIcon, title, to, ...rest }: NavButtonProps) => {
42+
// Use useMatch to determine if the current route matches the button's destination
43+
// This provides the same functionality as NavLink's isActive prop
44+
// Only applies to buttons with a to prop (but needs to be before any return statements)
45+
const match = useMatch({
46+
end: to === "/", // Only exact match for root path
47+
path: to ?? "",
48+
});
49+
// Only applies to buttons with a to prop
50+
const isActive = Boolean(to) ? Boolean(match) : false;
51+
52+
const commonButtonProps = useMemo<ButtonProps>(
53+
() => ({
54+
_expanded: isActive
55+
? undefined
56+
: {
57+
bg: "brand.emphasized", // Even darker for better light mode contrast
58+
color: "fg",
59+
},
60+
_focus: isActive
61+
? undefined
62+
: {
63+
color: "fg",
64+
},
65+
_hover: isActive
66+
? undefined
67+
: {
68+
_active: {
69+
bg: "brand.solid",
70+
color: "white",
71+
},
72+
bg: "brand.emphasized", // Even darker for better light mode contrast
73+
color: "fg",
74+
},
75+
alignItems: "center",
76+
bg: isActive ? "brand.solid" : undefined,
77+
borderRadius: "md",
78+
borderWidth: 0,
79+
boxSize: 14,
80+
color: isActive ? "white" : "fg.muted",
81+
colorPalette: "brand",
82+
cursor: "pointer",
83+
flexDir: "column",
84+
gap: 0,
85+
overflow: "hidden",
86+
padding: 0,
87+
textDecoration: "none",
88+
title,
89+
transition: "background-color 0.2s ease, color 0.2s ease",
90+
variant: "plain",
91+
whiteSpace: "wrap",
92+
...rest,
93+
}),
94+
[isActive, rest, title],
95+
);
96+
97+
if (to === undefined) {
98+
return (
99+
<Button {...commonButtonProps}>
100+
{pluginIcon ?? <Icon as={icon} boxSize={5} />}
101+
<Box {...commonLabelProps}>{title}</Box>
59102
</Button>
60-
</Link>
61-
) : (
62-
<NavLink to={to}>
63-
{({ isActive }: { readonly isActive: boolean }) => (
64-
<Button
65-
{...styles}
66-
_active={isActive ? { bg: "brand.solid" } : { bg: "brand.emphasized" }}
67-
// Override styles for active state to ensure proper colors
68-
_hover={isActive ? { bg: "brand.solid" } : { bg: "brand.emphasized" }}
69-
variant={isActive ? "solid" : "ghost"}
70-
{...rest}
71-
>
72-
<Box alignSelf="center">{icon}</Box>
73-
<Box fontSize="xs">{title}</Box>
103+
);
104+
}
105+
106+
if (isExternal) {
107+
return (
108+
<Link asChild href={to} rel="noopener noreferrer" target="_blank">
109+
<Button {...commonButtonProps}>
110+
{pluginIcon ?? <Icon as={icon} boxSize={5} />}
111+
<Box {...commonLabelProps}>{title}</Box>
74112
</Button>
75-
)}
76-
</NavLink>
113+
</Link>
114+
);
115+
}
116+
117+
return (
118+
<Button as={Link} asChild {...commonButtonProps}>
119+
<RouterLink to={to}>
120+
{pluginIcon ?? <Icon as={icon} boxSize={5} />}
121+
<Box {...commonLabelProps}>{title}</Box>
122+
</RouterLink>
123+
</Button>
77124
);
125+
};

airflow-core/src/airflow/ui/src/layouts/Nav/PluginMenuItem.tsx

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
import { Link, Image, Menu } from "@chakra-ui/react";
19+
import { Link, Image, Menu, Icon, Box } from "@chakra-ui/react";
2020
import { FiExternalLink } from "react-icons/fi";
2121
import { LuPlug } from "react-icons/lu";
2222
import { RiArchiveStackLine } from "react-icons/ri";
@@ -46,21 +46,22 @@ export const PluginMenuItem = ({
4646
const displayIcon = colorMode === "dark" && typeof iconDarkMode === "string" ? iconDarkMode : icon;
4747
const pluginIcon =
4848
typeof displayIcon === "string" ? (
49-
<Image height="20px" mr={topLevel ? 0 : 2} src={displayIcon} width="20px" />
49+
<Image boxSize={5} src={displayIcon} />
5050
) : urlRoute === "legacy-fab-views" ? (
51-
<RiArchiveStackLine size="20px" style={{ marginRight: topLevel ? 0 : "8px" }} />
51+
<Icon as={RiArchiveStackLine} boxSize={5} />
5252
) : (
53-
<LuPlug size="20px" style={{ marginRight: topLevel ? 0 : "8px" }} />
53+
<Icon as={LuPlug} boxSize={5} />
5454
);
5555

5656
const isExternal = urlRoute === undefined || urlRoute === null;
5757

5858
if (topLevel) {
5959
return (
6060
<NavButton
61-
icon={pluginIcon}
61+
icon={LuPlug}
6262
isExternal={isExternal}
6363
key={name}
64+
pluginIcon={pluginIcon}
6465
title={name}
6566
to={isExternal ? href : `plugin/${urlRoute}`}
6667
/>
@@ -80,13 +81,15 @@ export const PluginMenuItem = ({
8081
width="100%"
8182
>
8283
{pluginIcon}
83-
{name}
84-
<FiExternalLink />
84+
<Box flex="1">{name}</Box>
85+
<Icon as={FiExternalLink} boxSize={4} color="fg.muted" />
8586
</Link>
8687
) : (
8788
<RouterLink style={{ outline: "none" }} to={`plugin/${urlRoute}`}>
8889
{pluginIcon}
89-
{name}
90+
<Box flex="1" ml={2}>
91+
{name}
92+
</Box>
9093
</RouterLink>
9194
)}
9295
</Menu.Item>

0 commit comments

Comments
 (0)