thecodingtrain.com icon indicating copy to clipboard operation
thecodingtrain.com copied to clipboard

Added Dark Mode Feature to thecodingtrain Website

Open Yashasewi opened this issue 10 months ago • 34 comments

Dark Mode Implementation

This pull request introduces the implementation of the dark mode feature for the thecodingtrain.com website, as discussed in #1504.

Changes

  • Implemented dark mode using CSS variables and theming classes
  • Created a dedicated theme.js component to change CSS variables for light and dark modes
  • Updated existing styles to reference the appropriate CSS variables for theming
  • Addressed instances of hard-coded colors and updated them to use CSS variables

Implementation Details

CSS Variables and Theming

The theme.js file defines two sets of CSS variables: one for the light mode and another for the dark mode. These variables control various colors and styles throughout the website, including text, backgrounds, and UI elements.

/* Light Mode Variables */
const lightTheme = [
  '--text-color: #000000',
  '--gray-dark: #3c3c3c',
  '--gray-mid: #666666',
  ......
];

/* Dark Mode */
const darkTheme = [
  '--text-color: #f0f0f0',
  '--background-color: #1e1d20',
  '--gray-dark: #1f1f1f',
  '--gray-mid: #b3b3b3',
  ......
];

The theme.js component manages the application of the appropriate CSS variables based on the user's preference (light or dark mode). It also persists the user's preference in localStorage for consistency across sessions.

Existing Styles Updates

Existing CSS styles have been updated to reference the appropriate CSS variables, ensuring that colors and visual elements adapt seamlessly to the selected theme.

Potential Issues

  • While updating the styles to use CSS variables, there might be some instances where colors were hard-coded or missed. These instances could potentially lead to inconsistencies or unintended visual effects in the dark mode. Further testing and review from the team would be beneficial to identify and address such cases.
  • Additionally, the icons in the footer cannot be changed by simply adjusting the color, so we might need to use new icons or find an alternative solution to ensure they are visible and consistent with the dark mode.
  • Regarding the gradient present on the home page's homescene, I couldn't find a suitable alternative that works well with the dark mode. I have left this part for the design team to decide on a new gradient or alternative design element that looks good in the dark mode.

Next Steps

  • Address any feedback or suggestions from the review process
  • Conduct accessibility audits to ensure compliance with WCAG guidelines
  • Identify and address any remaining instances of hard-coded colors or inconsistencies
  • Please review the changes and provide your feedback. I'm happy to address any concerns or make further adjustments as needed.

Resolves #1504

@shiffman @runemadsen @jasontheillustrator

Yashasewi avatar Apr 14 '24 08:04 Yashasewi

Deploy Preview for codingtrain ready!

Built without sensitive environment variables

Name Link
Latest commit f97cc5d981d9d331c035a4a22d7e69ff136e46c7
Latest deploy log https://app.netlify.com/sites/codingtrain/deploys/663d51bd5441030008efd7d9
Deploy Preview https://deploy-preview-1554--codingtrain.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

netlify[bot] avatar Apr 14 '24 08:04 netlify[bot]

Thank you again @Yashasewi I am so excited to have this, it is amazing work! (And thank you for your patience, I know it can be frustrating to have contributions linger for a long time!).

I have reviewed the deploy preview and am happy with this! Does anyone have any remaining technical comments before I merge?

Also, I believe I found a bug, one some pages in dark model there are still "light" sections, for example:

https://deploy-preview-1554--codingtrain.netlify.app/tracks/data-and-apis-in-javascript

Screen Shot 2024-04-20 at 9 42 23 AM

shiffman avatar Apr 20 '24 13:04 shiffman

other inconsistencies/bugs:

(in the following screenshots kindly ignore font/text rendering issues, they are local to my machine due to weird resolution settings)

Footer social links - icon issue? image

Track card - language/topic tags not visible (url) image

Track video header - section with light background, language/topic tags not visible (url) image

VideoSection - timestamps not visible, section with light background (url) image

VideoInfo + PassengerShowcasePanel - links/description/credits not visible, passenger showcase header not visible, showcase name not visible (url) image

(Passenger showcase is correctly rendered on the showcase page, so probably the same styles need to be used on the video pages as well)

(same changes as above for "Related Challenges" section)

PassengerShowcaseForm - dark text (url) image

dipamsen avatar Apr 20 '24 17:04 dipamsen

Hi @shiffman and @dipamsen,

Thank you for taking the time to review the dark mode implementation and for your valuable feedback. I appreciate you pointing out the issues and inconsistencies you've encountered. (No worries, I know these things take time! 😄)

Regarding the bug you mentioned, @shiffman, where some pages in dark mode still have "light" sections (e.g., deploy-preview-1554--codingtrain.netlify.app/tracks/data-and-apis-in-javascript), I will investigate the issue and ensure that the dark mode styles are applied consistently across all pages.

As for the other inconsistencies/bugs reported:

  1. Footer social links - icon issue?
  • The current SVG icons used in the footer do not seem to support the dark mode styles effectively. Since these icons are in SVG format, simply adjusting the color might not work as expected. I think we need to consider using different icon sets or formats that support dark mode natively, or explore alternative solutions to ensure the icons are visible and consistent in both light and dark modes.

what should we do about the icons ? @shiffman any suggestions...

  1. Other Issues
  • Thank you for catching the other issues. I think I missed some pages on the website. But sit back and relax, I will fix the other issues soon.

@dipamsen, if you could share the specific URLs where you encountered the issues, it would be extremely helpful for me to locate and fix them easily.

I will work on addressing them and update the pull request accordingly. If you encounter any other inconsistencies or issues, please feel free to share them as well. I appreciate your collaboration, support, and understanding in ensuring a seamless integration of the dark mode feature.

Yashasewi avatar Apr 20 '24 18:04 Yashasewi

@Yashasewi I have updated my comment to include example urls to pages on the deploy preview site.

dipamsen avatar Apr 21 '24 02:04 dipamsen

Hi @shiffman and @dipamsen,

  • [x] Track card - language/topic tags not visible (url)
  • [x] Track video header - section with light background, language/topic tags not visible (url)
  • [x] VideoSection - timestamps not visible, section with light background (url)
  • [x] PassengerShowcaseForm - dark text (url)
  • [x] VideoInfo + PassengerShowcasePanel - links/description/credits not visible, passenger showcase header not visible, showcase name not visible (url)
  • [x] Footer Social Links - Icon Issue
  • As @dipamsen mentioned that the current SVG icons used in the footer do not handle dark mode styles effectively.
  • Proposed Solution: I can change the icons to new icons that support dark mode switching, ensuring consistent visibility and styling.

Regarding the footer social links, if I have the necessary permissions, I will proceed with changing the icons to new ones that support dark mode switching. This should resolve the icon visibility and styling issues in both light and dark modes.

I have fixed most of the bugs found during your review. However, if you encounter any additional issues or inconsistencies, please do not hesitate to mention them.

Yashasewi avatar Apr 21 '24 17:04 Yashasewi

amazing! one last place that i missed mentioning, is the challenge page which has some white areas, as well as dark text (example)

Also, the search results for tracks (url) and the date of video on the challenge page (url) could be light text?

dipamsen avatar Apr 27 '24 09:04 dipamsen

Hii @dipamsen Fixed most of the issues. Please check it and if you find any bugs or inconsistency please mention here.

Yashasewi avatar Apr 27 '24 14:04 Yashasewi

Looks great!!! After a check I found few more places:

  • About Page (url)
  • Track video page (url) - track stops/time stamps label not visible on hover image
  • Challenge page (url) - parts/time stamps label not visible (on active and hover) image

dipamsen avatar Apr 27 '24 16:04 dipamsen

Hii @dipamsen Please check it and if you find any bugs or inconsistency .... mention here. Also updated icons on footer .

Yashasewi avatar Apr 28 '24 08:04 Yashasewi

Hey everyone! I'm a bit late to the dark mode discussion, apologies. Great job @Yashasewi, looks like we're getting close!

Theme toggle

I have some recommendations on the implementation.

General approach

It's counter-intuitive, but with a React framework that pre-renders static content at build time and needs to hydrate, using Context and React state is not the ideal way to go. It will cause a kind of FOUC where the user will see the wrong theme flash on first render, which we can observe in your current solution.

Here's a better approach, which is how the React team implement their own dark mode on react.dev:

  • Inject some JS code in the head or at the top of the body (it should block rendering) that will handle the localStorage + setting a "dark" class in the html tag if needed. This should probably live in the Layout or Head component of this project. Tailwind has a nice and simple example in their docs (only look at the JS sample, we're obviously not using Tailwind here): https://tailwindcss.com/docs/dark-mode#supporting-system-preference-and-manual-selection

  • Always render both toggle buttons in JXS, but hide the one that shouldn't be currently displayed with CSS selectors that leverage the presence of the "dark" class in the html element.

  • The theme toggle buttons can just add/remove the "dark" class and update the localStorage value directly. Again, I would reference the Tailwind link previously mentioned.

This should fix the FOUC and will not cause React hydration warnings. Let me know if you need help. I could take this on in the PR directly or do a follow up later.

Theme CSS variables in CSS file

It would be more in line with the project to move the CSS variables to an actual global CSS file (variables.css) instead of having them in JS. You can then dedupe/merge the light theme variables and add a html.dark selector for the dark theme.

Button semantic

Maybe I'm missing something, but could we not use a button element instead of making a span behave like one? It would be nice to add a pointer cursor, hover state and title too.

Misc

Social icons in footer

This new icon for Nebula doesn't look like their logo, the rotation is wrong and the star has rounded points. Maybe try io5/IoStarSharp with transform: scaleY(-1); instead?

We can also get rid of the old SVG icons if they're not used anywhere else.

Train illustration

Am I the only one that finds the wheels on the train illustration particularly odd-looking against a dark background?

CleanShot 2024-04-28 at 17 00 24@2x

Styles

Have we properly tested on mobile, and also looked for regressions on the light theme yet?

I have not had the time to do so, but one thing that jumped at me was the gray background in the footer being gone:

Before After
CleanShot 2024-04-28 at 16 52 00@2x CleanShot 2024-04-28 at 16 51 46@2x

fturmel avatar Apr 28 '24 21:04 fturmel

@fturmel Thank you for offering to contribute directly to the dark mode implementation. I really appreciate your willingness to help improve the project.

Regarding the FOUC issue, I was researching solutions, and your recommended approach is perfect. If you're open to it, I would be extremely grateful if you could take on the implementation of the general approach for the dark mode toggle. . Your expertise in this area would be invaluable.

I will focus on addressing the other points you raised, such as:

  • [x] Moving CSS variables to a global file.
  • [x] Improving button semantics for the theme toggle.
  • [x] Fixing social icons in the footer and removing unused SVG icons.
  • [x] Thorough testing across devices and screen sizes, checking for light theme regressions. (I tested the deployed preview on my Nothing Phone 2 and found some bugs, which have been fixed. If someone finds any bugs on inconsistency, please mention it here)

@fturmel If you can contribute directly to the pull request, please feel free to do so. I'm happy to collaborate. Your contributions will significantly enhance the overall user experience, and it would be an excellent opportunity for me to learn from your expertise.

Thank you again for your feedback and willingness to contribute. I look forward to collaborating with you on this project.

Yashasewi avatar Apr 29 '24 18:04 Yashasewi

@Yashasewi sounds good, happy to help. It might be easier for me to also move the CSS variables as I make the toggle implementation changes. I should have some time to get this done at some point tomorrow.

There's a potential for conflicts if we work on this at the same time because things are intertwined a bit, so make sure you push any pending local commits on your end.

fturmel avatar Apr 29 '24 18:04 fturmel

@fturmel currently I don't have any local changes to push .. so please go ahead and push your changes

Yashasewi avatar Apr 29 '24 18:04 Yashasewi

@Yashasewi All good, I think you can pick things up from here!

fturmel avatar Apr 30 '24 18:04 fturmel

@fturmel Thank you so much for your contribution to the dark mode implementation. It has significantly improved the overall user experience and brought us closer to delivering a polished dark mode feature.

There is one more issue that I've been trying to fix but haven't been successful yet. I would really appreciate your help with this if possible.

The issue is related to the search/filter input field. When you try to search or filter on specific pages like thecodingtrain url or the challenges page, the input text color becomes dark, which doesn't contrast well with the dark background and tends to blend in.

I've tried different approaches to fix this UI bug using CSS, but I haven't been able to resolve it successfully. If you could take a look and provide a solution or fix it , it would be extremely helpful.

Here's a video demonstrating the issue: video link Also in image image

Yashasewi avatar May 01 '24 06:05 Yashasewi

@Yashasewi The react-select component is not super well documented for manual CSS styling, I agree it's not the most obvious thing to figure out. You have to inspect the generated markup and identify the correct CSS class names to hook into.

b882ca7 should take care of it.

fturmel avatar May 01 '24 12:05 fturmel

@fturmel Thank you again for your assistance with the search/filter input field issue in dark mode. I really appreciate your efforts in finding a solution.

You're correct that the react-select component's documentation on manual CSS styling is lacking, which made it difficult for me to identify the correct class names to target. Despite my attempts to style it from the CSS side, I couldn't achieve the desired result.

However, I noticed that when we type anything in the field, the text input still appears in a dark color, which doesn't provide enough contrast against the dark background.

But 9669bab this should fix this problem. ( Tried your approach to look for generated markup and identify the correct CSS class names to hook into)

Yashasewi avatar May 01 '24 17:05 Yashasewi

@all-contributors add @Yashasewi for code

shiffman avatar May 02 '24 13:05 shiffman

@shiffman

I've put up a pull request to add @Yashasewi! :tada:

allcontributors[bot] avatar May 02 '24 13:05 allcontributors[bot]

This is so fantastic and such a heroic effort, thank you to all involved! I'm ready to merge this! As far as I can tell everything seems resolved, but let me know if I should hold off if there are continued refinements needed? Also tagging in @runemadsen in case you want to have one last look in terms of design? huge ❤️ for this!

shiffman avatar May 02 '24 13:05 shiffman

@shiffman Not quite there yet, but very close. We need a bit more QA, fix the Nebula icon in footer and fix the background color regression in the footer light theme. Also remove the now unused previous social logo SVG files.

Let me put this in draft mode temporarily.

fturmel avatar May 02 '24 13:05 fturmel

I just went through the whole site and made some additional improvements. Ready for a last look and merge imo.

fturmel avatar May 02 '24 16:05 fturmel

@fturmel I had made similar local changes to address some of the remaining issues, but after you went through the entire site and made these commits, I don't need to push those local commits anymore. The improvements and fixes you've implemented cover the areas I had planned to work on.

I was also testing this site on mobile devices, and the bugs I found were related to the code examples and the navigation not closing on mobile when activated. However, you have fixed both of these issues with your recent commits. I couldn't find any other issues or problems on mobile devices.

I believe we can safely merge this pull request into the main branch. If someone found any issue or bug or inconsistency, please mention here.

Yashasewi avatar May 02 '24 17:05 Yashasewi

@Yashasewi Sorry I stole your thunder! It made sense to just power through and fix the small issues as I was QA'ing locally.

I feel bad because I did spend time writing detailed comments before so you could have some direction and work it out yourself 🤦

I'll be more patient next time! And again, good job on this PR.

fturmel avatar May 02 '24 18:05 fturmel

@fturmel 😂 There's nothing to feel bad about at all! Working alongside an experienced contributor like yourself has been an incredible learning opportunity for me. How about we add dark mode to the Nature of Code site next? That would be a great follow-up project .

Yashasewi avatar May 02 '24 18:05 Yashasewi

Thank you, wow! I see that the checks are not passing, let me know if there's something I should do or check into! @Yashasewi I love the idea of dark mode for nature of code, but the site and its design are still quite a bit in flux so it probably would make sense to wait until it's more settled and final. Feel free to poke around the repo and look at the issues and other discussion! I welcome contributions!

shiffman avatar May 03 '24 20:05 shiffman

Alright, all checks are green again. There must've been an outage / glitch on Netlify's end a few days ago.

fturmel avatar May 04 '24 15:05 fturmel

I am noticing that foreground text color on certain sections of the light theme site has been changed from #3c3c3c (--gray-dark) to #000000, is this intended?

Before After
image image

dipamsen avatar May 04 '24 15:05 dipamsen

Also, the color pair of foreground and background on the Guides page (as well as Showcase, About) may be difficult to read due to low color contrast (does not conform to WCAG AA), maybe we can use a different (lighter) shade/color theme for these pages for the light theme?

image

dipamsen avatar May 04 '24 16:05 dipamsen