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

Follow dark mode preference #30

Merged
merged 18 commits into from
May 13, 2024
Merged

Conversation

ntonnaett
Copy link
Contributor

This sets the the color scheme according to prefers-color-scheme (that follows the global dark mode setting in Gnome). You can still toggle the dark mode from the menu.

@terryzfeng
Copy link
Member

Awesome, can you run npm run format and npm run lint to pass code style build checks? Thanks!

@terryzfeng
Copy link
Member

Also curious, I am unable to test this on Windows and Mac. Would I be able to test this on a CCRMA linux machine?

@ntonnaett
Copy link
Contributor Author

You can set dark or light mode directly in your browser. In Firefox it's in the General tab under Website appearance. Automatic would be set by your desktop environment or operating system. In Chromium it's in Appearance -> Mode. Unfortunately setting it directly to light or dark is broken for me. The automatic part (Device) works though. Maybe it's only broken on Linux?!

@ntonnaett
Copy link
Contributor Author

I'm not sure when this was added in macOS and Safari, but there should be the same options as in Gnome and Firefox/Chrome.

@terryzfeng
Copy link
Member

terryzfeng commented May 6, 2024

Hmm interesting, I'm trying to switch light and dark mode settings on Chrome (Windows) but it doesn't seem to do anything. Maybe this only works on Linux or Gnome?

In the past, the IDE theme used to similarly read browser light/dark mode but I ran into the issue of if your OS was dark but you wanted a light IDE, what is the source of truth? If you manually set it to light, how should the theme load after refreshing the page? Because of this confusion, I removed reading the browser theme to be OS theme agnostic.

@ntonnaett
Copy link
Contributor Author

In Chromium it didn't work for me either. I think that's a browser bug because it even follows my desktop setting when I explicitly choose dark mode. I checked that with this github repo page as github implements this too. Be aware that Github's main page (not logged in) is always dark.

@ntonnaett
Copy link
Contributor Author

In the past, the IDE theme used to similarly read browser light/dark mode but I ran into the issue of if your OS was dark but you wanted a light IDE, what is the source of truth? If you manually set it to light, how should the theme load after refreshing the page? Because of this confusion, I removed reading the browser theme to be OS theme agnostic.

Oh, I see. I think the correct thing to do then would be to save the setting as a cookie and have the options dark, light, and system. We do the same as the browser itself.

@terryzfeng
Copy link
Member

Oh, I see. I think the correct thing to do then would be to save the setting as a cookie and have the options dark, light, and system. We do the same as the browser itself.

Gotcha, yeah we only save the cookie for light/dark, not yet system. That would be a good feature...

@ntonnaett
Copy link
Contributor Author

This mostly works now. Though removing the Event Listener doesn't work. So even if you chose dark or light mode it still follows your browser or desktop setting. Maybe you understand why this doesn't work.

@ntonnaett
Copy link
Contributor Author

Format and lint don't agree on the formatting of switch statements?

@ntonnaett ntonnaett force-pushed the dark_mode_preference branch from fa5f106 to 8c169a1 Compare May 6, 2024 21:20
@ntonnaett
Copy link
Contributor Author

ntonnaett commented May 6, 2024

Okay, I just didn't remove the event listener but returned early instead if colorPreference is not "system". This works now. It's rebased and ready to be merged. I could make the history more beautiful though. Or squash it?

index.html Outdated Show resolved Hide resolved
src/utils/theme.ts Outdated Show resolved Hide resolved
src/utils/theme.ts Outdated Show resolved Hide resolved
@ntonnaett
Copy link
Contributor Author

I've implemented what we talked about on the way home. What do you think about the wording?

@ntonnaett
Copy link
Contributor Author

The dark mode toggle is now gone when turning on system preference.
https://ccrma.stanford.edu/~ntonnatt/webchuck-ide/

@ntonnaett ntonnaett force-pushed the dark_mode_preference branch from 96f244d to 7b6d9d5 Compare May 12, 2024 06:13
@ntonnaett ntonnaett changed the base branch from main to dev May 12, 2024 06:15
@ntonnaett
Copy link
Contributor Author

Rebased to dev.

@terryzfeng
Copy link
Member

It all seems to work! Could you put ChucK time first in the view dropdown to save a label? This way dark mode would be at the end of the dropdown? Then we can just call the label Theme to be neutral?

@terryzfeng
Copy link
Member

I know you removed the button now but when it was system but did you like the disabled button look? You can add disabled:opacity-50 button class to the html for the button, and set themeButton.disabled = true. Not sure if you liked it this way or not. I think it could be a bit more helpful?

@ntonnaett
Copy link
Contributor Author

Obey to Terry's orders ✓
Update https://ccrma.stanford.edu/~ntonnatt/webchuck-ide/

I forgot about the disabled option. I think it's better.

@terryzfeng
Copy link
Member

LGTM

@terryzfeng terryzfeng merged commit c63c61a into ccrma:dev May 13, 2024
1 check passed
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.

2 participants