Aryaman-leo commented on code in PR #62172:
URL: https://github.com/apache/airflow/pull/62172#discussion_r2828997392
##########
airflow-core/src/airflow/ui/src/layouts/Nav/Nav.tsx:
##########
@@ -157,27 +168,35 @@ export const Nav = () => {
<Flex alignItems="center" flexDir="column" gap={1} width="100%">
<Box alignItems="center" asChild boxSize={14} display="flex"
justifyContent="center">
<Link title={translate("nav.home")} to="/">
- <AirflowPin
- _motionSafe={{
- _hover: {
- transform: "rotate(360deg)",
- transition: "transform 0.8s ease-in-out",
- },
- }}
- boxSize={8}
- />
+ {hasIconSrc && colorMode && !failedLoadingCustomIcon[colorMode] ? (
+ // eslint-disable-next-line
jsx-a11y/no-noninteractive-element-interactions
+ <img
+ alt="Airflow"
+ onError={() => setFailedLoadingCustomIcon((prev) => ({
...prev, [colorMode]: true }))}
Review Comment:
I implemented the logo override using the UI plugin framework because the
original issue description explicitly proposed enabling customization via
plugins, including configuration and fallback handling. That’s the direction I
followed while designing the solution.
Regarding the ESLint warning — attaching an event listener to a
non-interactive element was used specifically to handle the image load failure
and provide the required fallback behavior. I understand the accessibility
concern, and that was the simplest reliable approach I found in React to manage
the onError handling at the time. If there’s a preferred pattern that aligns
better with the project’s accessibility standards, I’m happy to refactor
accordingly.
Since logo customization is now handled via the core THEME configuration for
3.2.0, I got the thing.
thanks
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
