You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ryanahamilton (via GitHub)" <gi...@apache.org> on 2025/10/28 21:47:46 UTC
[PR] Refines the visual design, interaction, and accessibility of the global navigation [airflow]
ryanahamilton opened a new pull request, #57455:
URL: https://github.com/apache/airflow/pull/57455
Refines the visual presentation of the
| Dark mode - Before | Dark mode - After | Light mode - Before | Light mode - After |
|--|--|--|--|
| <img width="153" height="826" alt="image" src="https://github.com/user-attachments/assets/2cad0e51-8252-4a23-85eb-c738006fc2b3" /> | <img width="152" height="825" alt="image" src="https://github.com/user-attachments/assets/135bbcad-a7b4-4c13-9d1f-e9b8d1721ccc" /> | <img width="153" height="827" alt="image" src="https://github.com/user-attachments/assets/b455625a-0c0f-4b10-92b0-ab5dacc553a3" /> | <img width="152" height="826" alt="image" src="https://github.com/user-attachments/assets/24bf1f04-30ec-425f-b867-4865a8dea7ab" /> |
| Dark mode - Before | Dark mode - After | Light mode - Before | Light mode - After |
|--|--|--|--|
| <img width="280" height="214" alt="image" src="https://github.com/user-attachments/assets/ff8ec7e4-b537-4cab-a055-04123cc8d3df" /> | <img width="259" height="170" alt="image" src="https://github.com/user-attachments/assets/14abf53b-4f8b-43f2-ab96-6eba9f319bc2" /> | <img width="279" height="210" alt="image" src="https://github.com/user-attachments/assets/5c6418df-aad4-49dd-a911-a003389eb1b9" /> | <img width="263" height="171" alt="image" src="https://github.com/user-attachments/assets/93b67163-f8ae-43fe-becd-084bc86748e9" /> |
| Dark mode - Before | Dark mode - After | Light mode - Before | Light mode - After |
|--|--|--|--|
| | | | |
| Dark mode - Before | Dark mode - After | Light mode - Before | Light mode - After |
|--|--|--|--|
| | | | |
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "ryanahamilton (via GitHub)" <gi...@apache.org>.
ryanahamilton merged PR #57455:
URL: https://github.com/apache/airflow/pull/57455
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on PR #57455:
URL: https://github.com/apache/airflow/pull/57455#issuecomment-3459521190
This looks great. Love it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "ryanahamilton (via GitHub)" <gi...@apache.org>.
ryanahamilton commented on code in PR #57455:
URL: https://github.com/apache/airflow/pull/57455#discussion_r2471297679
##########
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")} />
Review Comment:
`NavButton`'s internal implementation of the `icon` prop was refactored so the size doesn't have to be defined at every implementation as it currently is.
##########
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 { NavLink, 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 ? (
Review Comment:
Refactored this without using a nested ternary to improve the legibility of the file.
##########
airflow-core/src/airflow/ui/src/layouts/Nav/UserSettingsButton.tsx:
##########
@@ -55,89 +55,88 @@ 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;
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>
+ </>
Review Comment:
Added the fragment wrapper so that the Modal components didn't need to be nested within `Menu.Root`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "ryanahamilton (via GitHub)" <gi...@apache.org>.
ryanahamilton commented on PR #57455:
URL: https://github.com/apache/airflow/pull/57455#issuecomment-3464225693
> Could we use at least a line break to have a text in two lines? Or is it always cropping first line?
@jscheffl It's intentionally always cropping to a single line. This is how we can enforce a consistent presentation across the navigation items, regardless of what value is entered in the translation file. Given there are only a handful of these nav items and they are persistently visible in the application, the icons really act as the primary identifiers since people can process symbols quicker than words and the labels are secondary.
FWIW, In your example, the label is `Datensets (Assets)`, which is combining 2 languages. I would argue that it should only be `Datensets`, which in turn removes the truncation for this language:
<img width="84" height="85" alt="image" src="https://github.com/user-attachments/assets/e00541f1-a7dc-4373-80d1-03cc43e10baf" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #57455:
URL: https://github.com/apache/airflow/pull/57455#issuecomment-3460976297
### Backport successfully created: v3-1-test
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-1-test</td>
<td><a href="https://github.com/apache/airflow/pull/57481"><img src="https://img.shields.io/badge/PR-57481-blue" alt="PR Link"></a></td>
</tr>
</table>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "ryanahamilton (via GitHub)" <gi...@apache.org>.
ryanahamilton commented on code in PR #57455:
URL: https://github.com/apache/airflow/pull/57455#discussion_r2472585377
##########
airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx:
##########
@@ -140,36 +140,35 @@ export const Nav = () => {
height="100%"
justifyContent="space-between"
position="fixed"
- py={3}
+ py={1}
top={0}
- width={20}
+ width={16}
zIndex={2}
>
- <Flex alignItems="center" flexDir="column" width="100%">
- <Box mb={3}>
- <NavLink to="/">
- <AirflowPin
+ <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
Review Comment:
Ah, I had used GitHub's browser conflict resolution—doesn't handle the easy stuff very well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #57455:
URL: https://github.com/apache/airflow/pull/57455#issuecomment-3464148885
Very cool... just one minor nit: As you had the example of german translation above with overflowing label... this is now hard to read. Could we use at least a line break to have a text in two lines? Or is it always cropping first line?
@TJaniF might we need / shall we adjust German translation for the change? WDYT
Also might affect other languages...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #57455:
URL: https://github.com/apache/airflow/pull/57455#discussion_r2472091272
##########
airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx:
##########
@@ -140,36 +140,35 @@ export const Nav = () => {
height="100%"
justifyContent="space-between"
position="fixed"
- py={3}
+ py={1}
top={0}
- width={20}
+ width={16}
zIndex={2}
>
- <Flex alignItems="center" flexDir="column" width="100%">
- <Box mb={3}>
- <NavLink to="/">
- <AirflowPin
+ <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
Review Comment:
```suggestion
<AirflowPin
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refines the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #57455:
URL: https://github.com/apache/airflow/pull/57455#issuecomment-3458849069
Nice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine the visual design, interaction, and accessibility of the global navigation [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on PR #57455:
URL: https://github.com/apache/airflow/pull/57455#issuecomment-3459191237
Love it ❤️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org