Skip to content

Implement TODO: enhance dark mode detection#373

Open
tageniu wants to merge 3 commits intoalibaba:mainfrom
tageniu:feat/dark-mode-detect
Open

Implement TODO: enhance dark mode detection#373
tageniu wants to merge 3 commits intoalibaba:mainfrom
tageniu:feat/dark-mode-detect

Conversation

@tageniu
Copy link
Copy Markdown

@tageniu tageniu commented Mar 30, 2026

What

Resolve the @todo in checkDarkMode.ts by adding 3 new dark mode detection strategies and expanding data-attribute coverage:

  • Data attributes: also check data-color-mode (GitHub), data-bs-theme (Bootstrap 5), data-color-scheme on both <html> and <body>
  • CSS color-scheme: read <meta name="color-scheme"> and the computed color-scheme property on :root
  • Layout containers: inspect background color of <main>, #app, #root, #__next, [role="main"] for SPAs where <body> stays transparent
  • Text color: treat high-luminance body text (> 180) as a dark-theme signal

Type

  • Bug fix
  • Feature / Improvement
  • Refactor
  • Documentation
  • Website
  • Demo / Testing
  • Breaking change

Testing

  • Tested in modern browsers
  • No console errors
  • Types/doc added

Requirements / 要求

  • I have read and follow the Code of Conduct and Contributing Guide . / 我已阅读并遵守行为准则。
  • This PR is NOT generated by a bot or AI agent acting autonomously. I have authored or meaningfully reviewed every change. / 此 PR 不是由 bot 或 AI 自主生成的,我已亲自编写或充分审查了每一处变更。

Copilot AI review requested due to automatic review settings March 30, 2026 22:05
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 30, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 computed color-scheme on :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.

Comment thread packages/page-controller/src/mask/checkDarkMode.ts Outdated
Comment thread packages/page-controller/src/mask/checkDarkMode.ts
Comment thread packages/page-controller/src/mask/checkDarkMode.ts Outdated
@gaomeng1900
Copy link
Copy Markdown
Collaborator

gaomeng1900 commented Apr 2, 2026

Thanks for your contribution.

Before I merge this PR. Please rebase and fix the following problems:

  • fix the commit message to fit conventional style Update packages/page-controller/src/mask/checkDarkMode.ts
  • replace the author admin with real username

@tageniu

@tageniu tageniu force-pushed the feat/dark-mode-detect branch from 2fbe15d to 032f9a7 Compare April 21, 2026 00:06
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
@tageniu tageniu force-pushed the feat/dark-mode-detect branch from 032f9a7 to 3206ae4 Compare April 21, 2026 00:14
@tageniu tageniu force-pushed the feat/dark-mode-detect branch from 3206ae4 to c6f0937 Compare April 21, 2026 00:32
@tageniu
Copy link
Copy Markdown
Author

tageniu commented Apr 21, 2026

@gaomeng1900 the two problems were fixed. sorry for the delay!

@tageniu
Copy link
Copy Markdown
Author

tageniu commented May 3, 2026

@gaomeng1900 hi there, I've fixed the problem you mentioned, could you review and merge the pr? Thank you in advance!

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.

4 participants