-
Notifications
You must be signed in to change notification settings - Fork 84
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 Darkmode (#766) #778
base: master
Are you sure you want to change the base?
Add Darkmode (#766) #778
Conversation
zeejot
commented
Oct 13, 2019
- improve settings/options
- add darktheme and switch in options
- update css file
* improve settings/options * add darktheme and switch in options * update css file
This PR changes all indents which makes it very hard to follow what the actual changes are. Would you want to clean up and also add a screenshot? Is it possible to detect dark mode from browser side on OSX instead of forcing it by user option? |
Automatic dark mode detection like in https://github.com/manuelottlik/ct-css-darkmode seems not to work. :-( |
Did you see https://stackoverflow.com/questions/50840168/how-to-detect-if-the-os-is-in-dark-mode-in-browsers#50843966. It would be nice to only have a manual setting when not running on OSX? |
Test Site for dark theme browser support: It's not working for Edge browser, but for Chrome etc. |
CSS variables only work as of Edge 15, released 2017. I guess that's acceptable? |
Jop and there is also the manual override. |
What happened to the "eye" icon (toggles channel visibility)? |
…into darkTheme # Conflicts: # htdocs/css/style.css
Good evening, found some bugs with links and margins that are fixed now. Do you think it makese sense to update the icons too? Maybe to fontawesome or something else? |
I would really love to replace our icons with an SVG icon font to get a consistent look and feel. Sofar I have not been able to find one that would especially delivery good symbols for all the different channel types. Long story short: a PR would be great, but should be separate from this one. |
Personally, I don't like the new striped table look (especially inside dialog boxes). I didn't have time yet to check the look on Mac but isn't the redesigned slider another, separate PR? |
I totally agree with you - the striped table look in the dialog boxes was ugly. I fixed that with last change. Do you mean the toggle switch with redesigned slider? |
Hi Andig, |
I tend to not merge this PR. A goal of UI refactoring was to get rid of the settings in an effort to make the UI cleaner. This PR introduces another setting. If it helps we could merge the CSS variables instead to make it easier for a fork to implement the dark theme. |
Most OS supports nowadays a dark mode. Like Android, iOS, Windows 10 also browsers like Chrome supports the dark mode. Some also time triggered. So for a good user experience also web pages should support the dark mode. Even if the dark mode auto detection does not yet work in all OS/Browser combinations this will for sure improve in future. @andig What did you mean with "This PR introduces another setting"? The toggle switch to manually enable/disable the dark mode? Or the line in options.js? I don't think they are a reason not to merge PR. Especially since they have been present since the beginning of PR. And zeejot invested a lot of work. |
The switch
This PR totally changes the appearance of the UI, even in normal mode, while claiming to "add" a dark mode. As such it is maximally invasise and won't get merged, at any rate as-is. |
I also hacked sth together... |