-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add dark mode feature #789
base: develop
Are you sure you want to change the base?
Conversation
amazing, thanks! |
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.
Amazing work!! It's looking great, thanks a lot!
I have a few thoughts on the design, let me know what you think:
- I think that we should be careful about not going too hard on the blue as almost everything is in currently blue (I'm suggesting some ideas in the following)
- The text color isn't very consistent throughout the website (sometimes white, blue and gray). I think homogenizing it to white may be more legible (and stand out more). In my opinion, the more important the text is (e.g. titles) the more it should stand out, e.g. the whiter it should be. Less important text can be darker shades of white and gray. This is up for debate of course, I'm not an authority on this:
- "stributed" and "llaborative learning" could be brighter (e.g. text-slate-300)
- Same thing for the text in the information page (the text in the next tabs looks great).
- Same for the text of the progress bars throughout the webapp. Also the active icon isn't put in orange in the dark theme
I think the TaskList is looking great:
-
I prefer the ButtonsCard title in white rather than light blue:
-
The federated and decentralized GIFs in the information page could keep a light background until they are compatible with a dark background. Right now they aren't legible. For example when adding
<FederatedGIF class="bg-slate-100 rounded-lg border-slate-300 border-4"/>
-
What do you think of doing something similar for the Disco logo until it gets handled (
<DiscoGIF class="bg-slate-100 rounded-lg pb-16 px-4"/>
. It's definitely not perfect but in my opinion it's better than the pixels
-
The ButtonsCard's shadow or border isn't displaying well (on Firefox at least) in the model evaluation page
-
I think the training charts aren't looking very good with the default settings in the dark theme. The text could be whiter too
…rk mode (in the information progress bar)
…strap class that is not usefull
…charts in order to make them display the new theme, so it doesn't make sense to use a ref here
Is the PR ready for review @tomasoignons? |
Yeah @JulienVig, i did all the requested changes I think |
Great! In general you can re-request a review via github when you're done addressing comments to let the reviewer know it's ready for another review round. |
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.
Very nice! I actually quite like how the federated and decentralized GIFs look like with the light background
There are a few things left to adapt to the dark theme, I've left comments through the code. Additionally could you add a light background to the MLO logo in the About Us page? I'm in contact with our graphic designer, we should get the new figures soonish.
While you're changing the UI can I ask you fix something I've noticed? The component displays too close to the next word sometimes, could you add a space where needed?
Co-authored-by: Julien Vignoud <[email protected]>
Co-authored-by: Julien Vignoud <[email protected]>
Co-authored-by: Julien Vignoud <[email protected]>
…orative progress bar
…o to add dark theme support for it
I added a dark mode feature, adding a dynamic button on the sidebar, and using tailwind dark mode class system.
Here are the previews :
Edit : also added dark theme in the testing, did'nt know they were custom components
What needs to be done to perfect :
Otherwise I am satisfied about how it turned, it may not be perfect, but it is better than nothing.
I also integrated a new sunIcon, taken from https://tablericons.com/, a really cool website with a lot of open source icons.
this pull request directly respond to the issue #387