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

ui: add dark mode toggle to navbar #2174

Merged
merged 26 commits into from
Feb 22, 2022
Merged

ui: add dark mode toggle to navbar #2174

merged 26 commits into from
Feb 22, 2022

Conversation

Forfold
Copy link
Contributor

@Forfold Forfold commented Feb 17, 2022

Closes #1694

Introduces a button group for dark mode to the user profile, defaulting to the system preference, saving to local storage. The primary grey color for light mode has been retained, with MUI defaults being leveraged for dark mode.

Component changes to accommodate dark mode:

  • Adds ThemePicker component to render option buttons from profile
  • Adds themeConfig.ts file containing a ThemeContext, and a ThemeProvider implementing the theme config, removing the old config within mui.ts
  • Alt white GoAlert logo when dark mode is active
  • Fixes search bar background when dark mode is active
  • Fixes a reference to the old theme obj in ScheduleCalendar, now that theme has moved
  • Fixes AlertListFilter icon to use proper contrast color when in dark mode
    • (Out of scope: default IconButton color to secondary)
  • Fixes the highlighted item text color in FlatList
  • Fixes FlatList list item subheader background color always being white (now transparent to match bg, no difference to light mode)
  • Fixes icon when in dark mode for OtherActions in RotationUserList if the user is active
  • Adds a white svg icon that the theme switches between for the Slack and SlackBW icons
  • Fixes type error from theme imports (after theme implementation changed) in several files
  • Removes light grey color for cancel buttons, in favor of the default primary color
  • Fixes error messages text color in dark mode

Out of Scope:

  • Choosing primary and secondary colors for light & dark mode
  • Changing the hard-coded colors like FlatList highlighted items, or SetFavoriteButton's heart color to better fit both light & dark mode palettes
  • Transitioning button and similar components to use secondary colors
  • Creating a settings dialog to improve user experience (see: ui: global settings dialog #1068)

Screenshots:

Screen Shot 2022-02-21 at 11 30 02 AM

Screen Shot 2022-02-21 at 11 30 17 AM

@Forfold Forfold marked this pull request as ready for review February 18, 2022 22:23
dctalbot
dctalbot previously approved these changes Feb 22, 2022
Copy link
Contributor

@dctalbot dctalbot left a comment

Choose a reason for hiding this comment

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

Next up should include:

  • Handling error screens (e.g. 404 sad face)
  • Handling secondary dialog buttons (e.g. cancel, close)

@Forfold
Copy link
Contributor Author

Forfold commented Feb 22, 2022

Good callout on the error screens- that's a good addition to add to our draft issue

@mastercactapus
Copy link
Member

mastercactapus commented Feb 22, 2022

@dctalbot Seems like something that got missed but caught in review. Hard to see, but hopefully an easy fix:
image

Secondary buttons aren't as bad, but that also looks like it's not intentional:

image

image

I think the secondary is supposed to have blue text matching the other button (I could be wrong)
image


We should fix things like that before merging, anyone with a dark system theme will get the dark mode by default so we should make sure it's finished, at least the simple stuff.

@Forfold Adjusting color for form dialogs (cancel/back buttons only) and error components shouldn't require major changes, correct?

@Forfold
Copy link
Contributor Author

Forfold commented Feb 22, 2022

The blue text is primary @mastercactapus, so probably not the best option for the cancel buttons. We have secondary as grey in dark mode for the purposes of consistency, but I can remove that in favor of the default (purple) if we want.

Edit: It looks like material.io uses primary for the cancel button too oddly, so that should be fine to go with

@mastercactapus
Copy link
Member

@Forfold Yeah, that's what I noticed in the MUI dialog examples:
https://mui.com/components/dialogs/

Since that's what we do in light mode I assumed dark mode would work the same, in any case, my concern is just the readability. Whatever you think is best

@Forfold
Copy link
Contributor Author

Forfold commented Feb 22, 2022

Well we don't actually have a primary or secondary color for light mode. We just have grey. So I was assuming that it should be secondary, but yeah since material.io (https://material.io/components/dialogs#anatomy) shows both buttons using primary too we can go with that

@mastercactapus
Copy link
Member

@Forfold sounds good I took a look locally, much better. What about the components in Errors.js IIRC those are just SVG and text, can we lighten those in dark mode?

As before, whatever you think is best, just looking for some better contrast

@Forfold
Copy link
Contributor Author

Forfold commented Feb 22, 2022

Yep, working through the error component right now

@Forfold Forfold merged commit e4c0abc into master Feb 22, 2022
@Forfold Forfold deleted the dark-mode branch February 22, 2022 19:12
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.

add dark mode toggle
3 participants