Added: Light mode toggle

drikusroor commented 1 year ago

This PR is basicaly a continuation of #64 but has been rebased onto the latest main branch and therefore also adds dark/light mode support for:

The new topic icons
The summary toggle button in the top bar

If we still want to add the theme toggle button, I would prefer to do that in a separate follow up PR.

UPDATE:

I've now converted the light mode feature from one that is based on a system preference by one that is based on a toggle button persisted in local storage.

Resolves #33

netlify[bot] commented 1 year ago

Deploy Preview for the-infi-way ready!

Name	Link
Latest commit	9873ee7f5c8723e488e6f0270a06be03a4aa7803
Latest deploy log	https://app.netlify.com/sites/the-infi-way/deploys/64e370741ec5700008a46250
Deploy Preview	https://deploy-preview-81--the-infi-way.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jeroenheijmans commented 1 year ago

I would prefer to do that in a separate follow up PR.

That seems fine to me, but I do still strongly prefer we create and merge that PR (into the branch for this PR) before we merge into main. Basically this comment from the earlier PR by David:

I think it's great we'll be offering a light mode version but agree that the combination of folks usually having a light themed OS and our website looking far better in dark mode is not great. Tailwindcss.com seems to have had the same consideration, and they default to dark mode with a toggle.

I think it's awesome and nice to offer a light mode, but The Infi Way looks so much better in Dark mode to me that I'd prefer to default it to that and making it fully about a toggle, instead of based on the operating system.

If y'all really do want to tie it into the OS setting, then I'd strongly prefer to work more on the Light Mode design and make it a bit more pleasing, on par with how good the Dark mode version is?

drikusroor commented 1 year ago

I think it's awesome and nice to offer a light mode, but The Infi Way looks so much better in Dark mode to me that I'd prefer to default it to that and making it fully about a toggle, instead of based on the operating system.

That's fair. I've implemented David's toggle button into the same branch in the end. Otherwise both branches would have been in my personal account and the PR would have to be done there.

jeroenheijmans commented 1 year ago

Cheers! Functionally the preview looks just about finished/good to me,

One small thing I think we could still fix in the PR: the dark-mode icons in "The Infi Test" (next to the numbers of the numbered list) are the same as the light mode icons, but the Black/White should be inverted. Compare the icons to the live way.infi.nl site to see what I mean, stroke color of the icons are white in those. To clarify (left is the PR):

part of "The Infi Test" with yellow arrow pointing out the visual change

The Deploy Preview also makes me realize that I'd like to do some follow-up issue to iterate some more on the design, especially of the top(right) bar, but I'll create an issue for that later on - certainly not for this PR I think.

PS. Haven't reviewed the code, hoping you can find someone in the daily standup to do that for us?

yougotdirked commented 1 year ago

noice

infi-nl / the-infi-way