Skip to content
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

webmail: add basic dark theme #163

Closed
wants to merge 1 commit into from
Closed

Conversation

mattfbacon
Copy link
Contributor

No description provided.

@mjl-
Copy link
Owner

mjl- commented May 3, 2024

hi @mattfbacon, i suppose you are interested in getting a dark mode merged?
at least the code change in this diff is small.

i don't normally use dark mode. but i tried this out. i noticed the white glow around the popups looked a bit strange. then i looked into what the internet says about designing a dark mode. seems that folks think it requires a bit more attention. another point raised (besides white glowing box-shadow) is stacking layers on top of each other (popups) works differently, and the layer on top should be lighter than the background layer (the current inversion doesn't do that, i tried it quickly, and it does look better that way). the colors for links also doesn't look too good to me.

have you considered making a more designed dark mode? the webmail has quite some inline styles, but not that many for colors. perhaps we can move some inline color styles to classes, so they can be styled differently in dark mode?

@mattfbacon
Copy link
Contributor Author

So, I looked at the inline styles to try to extract the colors to css variables but there were just too many for it to feel effective (e.g., 5 shades of background color is too much)e So I went with the simpler color manipulation approach. I have been using this patch locally and it fits my use case perfectly. But I would still be interested in a "better" version that has the colors more intentionally chosen.

mjl- added a commit that referenced this pull request May 6, 2024
… mode

this started with looking into the dark mode of PR #163 by mattfbacon. it's a
very good solution, especially for the amount of code. while looking into dark
mode, some common problems with inverting colors are:
- box-shadow start "glowing" which isn't great. likewise, semitransparent
  layers would become brighter, not darker.
- while popups/overlays in light mode just stay the same white, in dark mode
  they should become lighter than the regular content because box shadows don't
  give enough contrast in dark mode.

while looking at adding explicit styles for dark mode, it turns out that's
easier when we work more with css rules/classes instead of inline styles (so we
can use the @media rule).

so we now also create css rules instead of working with inline styles a lot.
benefits:
- creating css rules is useful for items that repeat. they'll have a single css
  class. changing a style on a css class is now reflected in all elements of that
  kind (with that class)
- css class names are helpful when inspecting the DOM while developing: they
  typically describe the function of the element.

most css classes are defined near where they are used, often while making the
element using the class (the css rule is created on first use).

this changes moves colors used for styling to a single place in webmail/lib.ts.
each property can get two values: one for regular/light mode, one for dark mode.
that should prevent forgetting one of them and makes it easy to configure both.
this change sets colors for the dark mode. i think the popups look better than
in PR #163, but in other ways it may be worse. this is a start, we can tweak
the styling.

if we can reduce the number of needed colors some more, we could make them
configurable in the webmail settings in the future. so this is also a step
towards making the ui looks configurable as discussed in issue #107.
@mjl-
Copy link
Owner

mjl- commented May 6, 2024

@mattfbacon While looking at dark mode, and doing some prototyping, I went all the way of using CSS classes for styling, and taking the inline style colors out into webmail/lib.ts, and adding a mechanism to easily add different colors for dark mode. This is a start, next steps would be to write some code that derives some of the colors (in darker/lighter shades) from a base color. Then we would also be close to making colors configurable. See commit message for details.

If new dark mode colors certainly aren't perfect, ideas to improve welcome.

@mattfbacon
Copy link
Contributor Author

Ok, as long as it supports prefers-color-scheme

@mjl-
Copy link
Owner

mjl- commented May 9, 2024

@mattfbacon yes, the darkmode styles (colors) are added with @media prefers-color-scheme dark.
i'll close this PR, thanks for sending it in. styling fixes could come in a new PR.

@mjl- mjl- closed this May 9, 2024
@mattfbacon mattfbacon deleted the dark branch November 23, 2024 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants