Implement TODO: enhance dark mode detection#373
Implement TODO: enhance dark mode detection#373tageniu wants to merge 3 commits intoalibaba:mainfrom
Conversation
Resolve the @todo in checkDarkMode.ts by adding 3 new detection strategies and expanding data-attribute coverage: - Check data-color-mode, data-bs-theme, data-color-scheme attributes - Read CSS color-scheme property and <meta name="color-scheme"> tag - Inspect background of SPA containers (#app, #root, #__next, etc.) - Detect light text color as a dark-theme signal
|
admin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Pull request overview
Enhances dark mode detection in isPageDark() by adding additional heuristics beyond class and background checks to better handle modern frameworks and theming approaches.
Changes:
- Expanded dark-mode detection via additional HTML/body data attributes.
- Added detection via
<meta name="color-scheme">and computedcolor-schemeon:root. - Added layout-container background inspection and a light-text heuristic as additional signals.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (meta) { | ||
| const content = meta.content.toLowerCase() | ||
| // "dark" or "only dark" → dark; "light dark" is ambiguous so skip | ||
| if (content === 'dark' || content === 'only dark') return true | ||
| if (content === 'light' || content === 'only light') return false | ||
| } |
There was a problem hiding this comment.
meta.content is lowercased but not trimmed before exact comparisons. Valid/meta-authored values like " dark " or "only dark " will currently be treated as inconclusive. Consider applying .trim() (and ideally normalizing internal whitespace) before comparing tokens.
| if (!el) continue | ||
|
|
||
| const style = window.getComputedStyle(el) | ||
| if (isColorDark(style.backgroundColor)) { | ||
| return true |
There was a problem hiding this comment.
isMainContentDark() relies on isColorDark(style.backgroundColor), but isColorDark/parseRgbColor ignore the alpha channel. Semi-transparent backgrounds like rgba(0, 0, 0, 0.4) will be classified as "dark" even though the effective blended background might be light, and formatting variations like rgba(0,0,0,0) can slip past the current transparency guard. Consider parsing alpha and treating alpha=0 as transparent (skip) and alpha<1 as inconclusive or walking up to a non-transparent ancestor/background.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thanks for your contribution. Before I merge this PR. Please rebase and fix the following problems:
|
What
Resolve the @todo in
checkDarkMode.tsby adding 3 new dark mode detection strategies and expanding data-attribute coverage:data-color-mode(GitHub),data-bs-theme(Bootstrap 5),data-color-schemeon both<html>and<body>color-scheme: read<meta name="color-scheme">and the computedcolor-schemeproperty on:root<main>,#app,#root,#__next,[role="main"]for SPAs where<body>stays transparentType
Testing
Requirements / 要求