New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ADD prototype version dark mode for Airflow UI #39355
base: main
Are you sure you want to change the base?
Conversation
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see dark mode feature. The screen shot that you shared doesn't show the text properly. is it during switch mode. are there any other screenshots?
Hi @dirrao The captured screen contains sensitive information, so we've mosaicked it, thank you for your understanding! :) And |
So in the dark mode screenshot, text is actually white (like in the nav bar) and it being very extremely low contrast is just a side effect of the mosaic? It would be helpful if there’s a non-filtered example. I have some worry about the blue used in links, for example. |
The screenshots illustrate that this will certainly be an appealing feature—I really wish the execution were as easy as this… The path that we should take (IMO) to enabling color mode in a sustainable manner is to migrate the rest of the UI to React. Within the React app, Chakra, the UI framework being employed, already has a robust color mode support baked in. The problem is that the React app is nested within the aged Flask+Bootstrap app and the two frameworks don't share a common color mode system. I'm sure it would be possible to bridge the divide/logic between them, but one would have to determine if that is worth the effort versus investing time in migrating more/the remaining UI to the React app. Once the UI is entirely in React, enabling it will trivial. |
hi @uranusjr. I removed the mosaicked part and created a test DAG to recapture it :) |
I am 80% okay with this interim. Anyway when moving to full-React we need to rework this thing. Found no problem on all existing pages with Firefox+Chromium on Ubuntu. What irritates me is really that flickering during page load. I also checked with PR #39345 merged-in but this also did not help. Can this be improved? I checked when setting my browser (Firefox as well as Chromium) to Dark mode explicitly, I'd expect the browser setting initially is picked-up. It is not. Can you add this? |
Hello, @jscheffl I fix the flickering problem. for this solution, I add custom function in views.py. It just a prototype, so please excuse the messy code. Add. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the re-work. Flickering is gone now. Almost looks good for me but I was clicking through the UI with the changes and found small new problems:
- Login page is not displaying in dark mode. Is always light
- Pages below menus "Security", "Browse" and "Admin" are all always in Light mode
Otherwise all "major" content looks good.
I see you added auto-detection of browser preference, but also via clearing cookies was not able to have this working. Was always showing light mode.
@jscheffl I initially thought that I would only need to modify the airflow/www side, but for "security", "browse", and "admin", modifying views.py would not fix the problem. This would require modifying the flask_appbuilder side, which I couldn't do. So, I solved this by deleting the custom classes I added to views.py and simply adding the script tag to the head of main.html. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a re-test on Chromium. Now all pages are consistent Dark or Light.
But "Flickering" is back, especially on the DAG overview. Both on Chromium as well Firefox in my test. Is this intended? I believe it was better before the last change.
@jscheffl However, i realized that it is currently not possible to modify the html of flaks_appbuilder within the airflow file. Therefore, I adjusted the position of the theme switching script syntax to optimize the flickering phenomenon. I've tested it locally with chrome and had no issues, but if you experience flickering with this revision, please let me know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did manual tests... cool: No flickering, consistent UI.
Good to be merged.
Oh... I'm so glad to hear that! :) |
@bbovenzi looking for a second reviewer, you also had an opinion on the PR? |
related: #11334
We've been seeing requests for a dark mode feature in the Airflow UI for a while now.
I tested it using docker-compose.yaml.
After testing, I was able to set dark mode and light mode.
The only thing I noticed is that after switching to dark mode, the light theme is visible for a very short time when the page is updated.
I think there should be a way to smooth this out in the future.
As-Is
To-Be
Dark Mode
Light Mode
You can toggle the dark/light mode by clicking the crescent icon on the right side of the navigation bar.
If you changed the color of the navigation bar via a setting like AIRFLOW__WEBSERVER__NAVBAR_COLOR, the dark/light mode will be applied accordingly.
FIX