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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement dark mode switch #2323
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @oyenuga17! Thanks for the contribution! This looks very nice.
Here's a few issues:
- ESC key cannot be used to close the modal
- clicking outside the modal does not close the modal
I think that having a HTML/JS/CSS modal here is probably too complex for what is needed here. It will be much simpler to use the browser's built in confirm()
method (see https://developer.mozilla.org/en-US/docs/Web/API/Window/confirm?retiredLocale=de) to obtain confirmation from the user.
Alternatively, we can update the privacy policy to list that using the dark/light mode switch will use LocalStorage to remember the user's setting.
hmm... you're right @sabine, might be hard to achieve a fully accessible modal using this my approach. I'll try the window.confirm method since that's the closest thing to what we're trying to achieve. |
@sabine I just mad the change. Please help review again |
Thanks @oyenuga17, I did some refactoring with the intent to improve code clarity. 鉂わ笍 Let's merge this now! |
Thank you @sabine 鉂わ笍 馃殌 |
Hi @sabine so i played around with this over the weekend and I managed to pull it off.
Here, is a video of the implementation. Please let me know what you think 馃槉