refactor(theme-controller): Document and simplify theme application logic#2115
Merged
rkaraivanov merged 4 commits intomasterfrom Feb 26, 2026
Merged
refactor(theme-controller): Document and simplify theme application logic#2115rkaraivanov merged 4 commits intomasterfrom
rkaraivanov merged 4 commits intomasterfrom
Conversation
…ogic - This commit cleans up some of the internal logic of the ThemingController, particularly around how themes are applied when context changes. It removes some redundant state management. - The commit also adds documentation of the ThemingController class and the exported factory function, explaining their purpose and usage.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors and documents the ThemingController class to improve code clarity and maintainability. The changes focus on simplifying theme application logic by consolidating redundant code paths and adding comprehensive JSDoc documentation.
Changes:
- Added detailed JSDoc documentation for the
ThemingControllerclass andaddThemingControllerfactory function - Refactored theme application logic by extracting common code into a new
_applyThememethod - Simplified
_adoptStylesmethod by removing the intermediate_getStylesmethod
…ogic Simplifies the theme provider and controller lifecycle management to improve reliability when components are dynamically added or moved in the DOM tree. Theme Provider Changes: - Move ContextProvider initialization from constructor to class field initializer - Remove constructor entirely (no longer needed) - Remove firstUpdated lifecycle method that redundantly set provider value - Consolidate theme/variant updates in the update() lifecycle method Theming Controller Changes: - Simplify hostConnected to always apply global theme first - Context providers synchronously invoke callbacks before first update cycle - This ensures correct theme resolution for dynamically created components - Add proper state cleanup in hostDisconnected: - Reset _themeSource to 'uninitialized' - Clear _contextConsumer value - Allows components to properly re-resolve theme when reconnected elsewhere - Improve inline documentation explaining the theme resolution logic Test Improvements: - Add comprehensive variant assertions throughout all test cases - Enhance component reconnection test with proper validation: - Tests moving component from inside provider to outside - Tests moving component back inside provider - Verifies theme source switches correctly in both directions These changes ensure that components correctly pick up theme context when: - Created inside an existing theme provider - Moved between different parts of the DOM tree - Moved in or out of theme provider scope
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of Change
Checklist