Skip to content

fix(react): reflect system theme in toggle icon - #1013

Open
mgalore wants to merge 1 commit into
nodejs:mainfrom
mgalore:fix/903-theme-toggle-icon
Open

fix(react): reflect system theme in toggle icon#1013
mgalore wants to merge 1 commit into
nodejs:mainfrom
mgalore:fix/903-theme-toggle-icon

Conversation

@mgalore

@mgalore mgalore commented Aug 8, 2026

Copy link
Copy Markdown

Summary

  • render the light or dark icon when the saved preference is system
  • keep the icon synchronized with operating-system theme changes
  • add focused resolution coverage and a patch changeset

Fixes #903

Validation

  • npm test
  • node --run test:ci
  • node --run lint
  • node --run format:check

Signed-off-by: Jonathan Amponsah 82057176+mgalore@users.noreply.github.com

Signed-off-by: Jonathan Amponsah <82057176+mgalore@users.noreply.github.com>
@mgalore
mgalore requested a review from a team as a code owner August 8, 2026 06:17
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api-docs-tooling Ready Ready Preview Aug 8, 2026 6:18am

Request Review

* Theme switcher.
*/
const Toggle = () => {
const [themePreference, setThemePreference] = useTheme();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this seems like an unrelate rename.

* @param {boolean} systemSupportsDarkMode - Whether the OS currently prefers dark mode.
* @returns {'light'|'dark'} The theme currently displayed by the page.
*/
export const getDisplayedTheme = (preference, systemSupportsDarkMode) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not a react hook.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@avivkeller I'm fairly sure this can be done on ui-components side? I thought this was done over there already, wondering if a change here is really needed. TRhe solution also doesn feel elegant...

@AugustinMauroy

Copy link
Copy Markdown
Member

also it's not seem fixing the issue

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.

theme switcher initial state for icon is wrong

3 participants