-
Notifications
You must be signed in to change notification settings - Fork 71
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
adds OS controlled dark mode #383
Conversation
I'll give it a proper test soon! As far as screenshot-based feedback goes, I have a couple questions:
I'm not at all an expert in design, please answer my questions in ELI5 mode if possible :-) I'll also test it with the GNOME dark mode setting and Firefox - I don't have a Mac handy to test with, but that's what you're using, so this gets us a bit more coverage at least. Thank you! |
Hi @david-christiansen, thanks for preliminary feedback. You brought up some great points. I agree that the dark mode headers are too dark and the text is too bright. I made the headers brighter (by eye) and the text a bit darker (to match Github) so that feels a little better. In terms of finding a balance, it's a tad tricky b/c accessibility guidelines recommend high contrast over low but dark users tend to abhor high-contrast UI 😅. I quite like the pink links but I'm not married to any of these colors so we can tweak as needed. That shade of pink was lifted from how hackage styles links. Re visited vs unvisited links -- my understanding is that the different link states exist for 2 interconnected reasons.
Final thoughts: note that I've adding instructions for testing the theme with Firefox and I wanted to clarify that I have not tampered with the light theme at all. It looks like the syntax highlighting is coming from highlight.js so picking out a different theme would be a separate work issue if you wanted something prettier? 😃 |
That part I could have figured out, but thanks!
Right - thanks! It is in-scope if necessary though, the visual design of the site is mostly a placeholder for something beautiful. It does the job, but it's clearly the result of some quick patches on my initial highly-minimalist CSS that I threw together in very little time. The Highlight.js issue seems to be a configuration one - I hadn't notice the issue before looking at screenshots here. I agree that it's a separate issue. |
Ah, I just saw that Hackage has dark mode - perhaps the best thing to do is to just take their design and colors and run with it. |
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.
Thanks for your patience!
I'm far from an expert in CSS, so please take any suggestions with a grain of salt. Generally speaking, I really like the use of variables to structure things and make it easier to maintain over time. Variables have only been supported since 2017 in Edge, but Firefox has supported them since 2014 and most other browsers since 2016. I suppose six years is long enough to wait, Haskellers are demographically likely to have updated browsers, and the HTML is semantic enough that the whole thing should be reasonably readable with defaults.
Generally speaking, the whole CSS code became easier to read with these changes.
I left some detailed comments in the code. Thanks again!
sounds good! Co-authored-by: David Thrane Christiansen <[email protected]>
Looks good Co-authored-by: David Thrane Christiansen <[email protected]>
…increases error-message text size.
Will merge as soon as CI is green. Thank you very much! |
🥳 |
Changes
How to Test
stack run -- site watch
http://localhost:8000/
Related Issue
Screen Shot