-
Notifications
You must be signed in to change notification settings - Fork 76
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
Dark mode #378
Dark mode #378
Conversation
This looks great, wow. A few notes:
|
Defaulting to dark mode seems to work well (I'm looking at https://deploy-preview-378--bisq-website.netlify.app/). One nit is that when dark mode is active, the menu toggle reads 'Dark', and when light mode is active, it reads 'Light'. Shouldn't this be the other way around? i.e. an affordance should advertise the state change that will occur if you make use of it (click it), as opposed to advertising the current state. Also, a common idiom I've seen is the use of a crescent moon symbol in the upper right for toggling dark / light mode. It takes up less real estate and is somehow more intuitive (to me) than just a textual 'light/dark' link. |
Just want to say I've been watching this PR -- it looks amazing! I see the markets dropdown text has been fixed, but the screenshots on the front page are still in light mode. Also I have agree with cbeams' comment above about flipping the dark mode toggle action and considering symbols instead of text for the label. |
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.
I had another look and everything seems to work fine. It looks so good (!).
I've attached a couple of nitpicks in the JavaScript...there's a stray console.out()
still there from debugging and an unused condition in 2 of the if statements.
That's it from my end. @pedromvpg I will ACK once you make those changes, and merge a short while afterward if there's no other feedback.
Co-authored-by: m52go <[email protected]>
Co-authored-by: m52go <[email protected]>
Co-authored-by: m52go <[email protected]>
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.
ACK
Add darkmode functionality and new client screen images.