[2.x] feat: add dark mode logo support#4457
Conversation
imorland
left a comment
There was a problem hiding this comment.
Nice feature, clean implementation. A few notes:
Change request:
The dark logo <img> has no base display: none rule, so browsers without :has() support (notably Firefox < 121) will render both logos simultaneously. Adding:
.Header-logo--dark-mode {
display: none;
}as a base rule would give correct graceful degradation — those browsers would simply always show the regular logo.
Notes (non-blocking):
- The dark logo is not used in HTML mail templates — email dark mode support is too inconsistent for this to be practical, just worth documenting as a known limitation.
[data-theme^='dark']correctly coversdark-hc(dark high contrast), so high contrast users get the dark logo. No separate variant is possible, but that's a reasonable limitation for now.- No tests, consistent with the existing light logo situation — not a blocker.
|
@imorland So in browsers without |
imorland
left a comment
There was a problem hiding this comment.
Apologies for the confusion — you're right, the base rule is already there. Change request was incorrect. LGTM!
imorland
left a comment
There was a problem hiding this comment.
Apologies for the confusion — you're right, the base display:none rule is already there. Change request was incorrect. LGTM!
Changes proposed in this pull request:
Not every logo works well in both light and dark mode. For example:
This PR adds an option to set an alternative logo for dark mode.
If no dark mode logo is set, the regular logo is used in dark mode.
Screenshot
Necessity
Confirmed