Skip to content

[dev-v5][Theme] Fix data-theme for light and system values#4766

Open
MarvinKlein1508 wants to merge 2 commits intomicrosoft:dev-v5from
MarvinKlein1508:fix-theme-from-body
Open

[dev-v5][Theme] Fix data-theme for light and system values#4766
MarvinKlein1508 wants to merge 2 commits intomicrosoft:dev-v5from
MarvinKlein1508:fix-theme-from-body

Conversation

@MarvinKlein1508
Copy link
Copy Markdown
Collaborator

Pull Request

📖 Description

This PR fixes an issue within the theme detection. The cases where the user has set data-theme="light" or data-theme="system" on the body were previously ignored by the initialization process.

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new component
  • I have modified an existing component
  • I have validated the Unit Tests for an existing component

⏭ Next Steps

Copilot AI review requested due to automatic review settings April 30, 2026 09:25
@MarvinKlein1508
Copy link
Copy Markdown
Collaborator Author

See #4628

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

Fixes theme initialization so data-theme="light" and data-theme="system" on <body> are honored during startup, instead of being ignored.

Changes:

  • Expand tryGetThemeModeFromBody() to recognize light and system in addition to dark.
  • Validate the data-theme attribute value against allowed modes before using it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Core.Scripts/src/Utilities/Theme.ts Outdated
Comment thread src/Core.Scripts/src/Utilities/Theme.ts Outdated
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