Skip to content
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

Implement Dark Mode Toggle #372

Merged
merged 10 commits into from
Dec 4, 2023
Merged

Implement Dark Mode Toggle #372

merged 10 commits into from
Dec 4, 2023

Conversation

markrickert
Copy link
Collaborator

Summary

This implements a dark mode toggle in the settings popover.

I was on a plane and had the upgrade helper open working on an upgrade and the brightness of the app was blinding me in a dark plane cabin. So I decided to work on implementing a dark mode toggle button. Hopefully this closes #97.

This was a two-fold approach using the included antd library's ConfigProvider to automatically control the colors of most buttons and text, and @emotion/react's ThemeProvider and useTheme() hook. The ThemeProvider also gave us access to style the components using styled CSS. I switched a few components over to antd to make them style automatically.

There is a new theme file included with two objects that should mirror each other and transferred over a lot of the existing colors for light mode and then used Github's darkmode as inspiration to fill out the dark mode object. I'm completely open to changing these colors if someone with a lot more design sense wants to weigh in.

Part of the theme object is theme.diff which lays out the colors for the html generated by react-diff-view. They have an example of darkmode colors in their example code, but it's commented out and it seems like they won't be implementing a dark mode in their library.

From that object, I applied the color overrides that had already been established (our dark green is a little nicer than the one from the library)

I've done my best to ensure that almost all instances of hard coded colors have been replaced by an entry in the theme file but i may have missed some places.

The dependencies already include use-persisted-state so i used that library to remember the user's choice of dark mode or not across browser sessions.

Test Plan

Here's a quick demo of the changes:

111B64AC-6833-43E9-B50F-AD62750F20AE-61572-000057CBE390DF60

And some screenshots:

Full Page:

Screenshot 2023-12-01 at 1 21 13 PM

Done Section:

Screenshot 2023-12-01 at 1 21 27 PM

Binary Downloads:

Screenshot 2023-12-01 at 1 22 06 PM

Inline Comments:

Screenshot 2023-12-01 at 1 22 31 PM

Checklist

  • One place to change all colors in the app.
  • Dark mode toggle switch in the settings panel.
  • Remembers your dark mode preference across web browser sessions
  • I tested this thoroughly
  • I added the documentation in README.md (if needed)

@lucasbento
Copy link
Member

This is amazing @markrickert, can't believe this is finally happening after 4 years since @pvinis opened that issue 😄
Thank you so much for working on this!

I took the liberty of tweaking the colors slightly, please do let me know what you think about the changes below, I followed the GitHub's scheme.

Before After
image image
image image

I would also suggest making the dark theme button more visible on the side of the config button, what do you think?

Again, thank you for working on this!

@markrickert
Copy link
Collaborator Author

@lucasbento Thanks for reviewing this! There's nothing more powerful than Outrage Driven Development, haha. The white background made it basically impossible for me to work on the dark airplane.

I like the tweaks you made, especially to the main background. Post your theme file here and I'll integrate it into my branch.


Do you think the button to toggle Light/Dark is important enough to add a third button at the top like this? I'd like to stick with already-existing dependencies, but I can't really find an appropriate symbol in the antd icons package. Any suggestions for an icon if you think this route is the way to go? I agree that it's a bit too hidden in the settings popover.

EB806F8F-FFEC-4219-BCEC-431A4B0E0924-14425-00003A349D6E46E0

@lucasbento
Copy link
Member

@markrickert hahaha that's amazing! I took the liberty of pushing the changes to your branch.

About the placement, I believe it's fine to put there, at least for the time being. People have been burning their eyes on light mode for four years, they will be happy to see your work right there!
As for the icon, how about using emojis? Should give a clean look 😄

@markrickert
Copy link
Collaborator Author

OK, the button is now at the top level and has a tooltip. ☀️ icon for light mode and 🌙 for dark mode.

Screenshot 2023-12-03 at 8 35 58 AM Screenshot 2023-12-03 at 8 35 52 AM

Copy link
Member

@kelset kelset left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mark! Thanks for working on this, I'm sure a lot of people will love that now they will be able to switch theme 👏

I tested it locally and it seems to be working well for me, and the code changes LGTM (but damn that Home component could use a refactor 🤣🙈)

@pvinis
Copy link
Member

pvinis commented Dec 4, 2023

Amazing!! Thank you for this work @markrickert. Merging!

@pvinis pvinis merged commit 8faec57 into react-native-community:master Dec 4, 2023
1 check passed
@markrickert markrickert deleted the mrickert/darkMode branch December 4, 2023 15:59
@markrickert
Copy link
Collaborator Author

Now that this branch is merged I may try working on a refactor and typescript conversion. I'll have some down time over the Christmas holiday to work on this

@markrickert
Copy link
Collaborator Author

I just visited the live site from my phone and it light mode was showing up as dark most and the toggle never changed it out of dark mode. I'll have to investigate this when I get home later.

@kelset
Copy link
Member

kelset commented Dec 4, 2023

I just visited the live site from my phone and it light mode was showing up as dark most and the toggle never changed it out of dark mode. I'll have to investigate this when I get home later.

I just tried on iOS on both Safari and Firefox and everything is working as expected.

@markrickert
Copy link
Collaborator Author

@kelset I'm using Brave Browser on an Android device. Light mode seems to be broken there somehow. Note the "sun" emoji, and clicking the button doesn't toggle the mode.

I'm glad to hear that it's working in more common browsers!

@kelset
Copy link
Member

kelset commented Dec 5, 2023

ok, thanks Mark!
I just tried on an Android phone and in Chrome and Edge it both works fine. So I think that it's probably an edge case of your local setup - and since at the end of the day I hope that noone in their right mind would try using the upgrade helper from their phone (😅), I think we could open a dedicate issue as a bug report but consider it low prior?

@markrickert
Copy link
Collaborator Author

markrickert commented Dec 5, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dark mode
4 participants