-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Implement an initial dark mode #11877
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
Conversation
Visit the preview URL for this PR (updated for commit 0766125): https://flutter-docs-prod--pr11877-feat-dark-mode-toggle-tcfsaj35.web.app |
This looks great. I checked about 50 pages and focused on as many UI elements as I could list. The only thing that stuck out was images. Here's one example where the background going dark makes it very hard to see the : https://flutter-docs-prod--pr11877-feat-dark-mode-toggle-tcfsaj35.web.app/ui/layout#conceptual-example I made a list of .md files that include an image reference: https://gist.github.com/lamek/0d635c930d29f4fbc72e628e93e8e14b A lot to check. Alternatively, just scanning the images dir for a file count: 86 directories, 879 files But, I wouldn't consider checking every file a blocker. IMO, spot checking ~50 images, only 1 had an issue. I think it might be best to tackle this over time, and add some quick guidance when adding an image to preview in both dark/light mode. |
"Regular" DartPad has a dark mode. Do you know if it's possible to toggle embedded DartPad to dark mode? Example page: https://flutter-docs-prod--pr11877-feat-dark-mode-toggle-tcfsaj35.web.app/cookbook/lists/floating-app-bar |
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.
Gorgeous!!!
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 your thorough review, @lamek! We discussed many of these issues in our 1-1-1 meeting today. :) As I stated in the meeting, I feel that this is good enough to land! lgtm
Resolves #3632