Repository navigation
Load the active theme on the settings and admin pages - #168
Merged
ralflang merged 1 commit intoOct 14, 2026
Merged
Conversation
The OAuth account, OAuth provider, OAuth status, API registry and admin dashboard controllers built their stylesheet URLs as "$themesUri . '/default/…'", so these pages always rendered with the default theme, and never loaded the scripts a theme declares in $theme_scripts. Use ViewModeConfigurator::addThemeStylesheets() and addThemeScripts() (horde/Core), which resolve the user's theme and walk the regular asset cascade. Themes that do not ship settings.css or admin-dashboard.css keep inheriting the default ones.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The declared Core dependency still permits stable versions that lack the newly called methods.
5 open findings
What changed in this PR
Updates settings and admin pages to load assets from the active theme.
Changes:
- Replaces hardcoded default-theme stylesheets with theme-aware discovery.
- Loads active-theme scripts after core scripts.
| File | Description |
|---|---|
src/Settings/OAuthAccountController.php |
Loads themed OAuth account assets. |
src/Admin/OAuthSystemStatusController.php |
Loads themed status-page assets. |
src/Admin/OAuthProviderController.php |
Loads themed provider-page assets. |
src/Admin/ApiRegistryController.php |
Loads themed API registry assets. |
src/Admin/AdminDashboardController.php |
Loads themed dashboard assets. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
| $themesUri = $this->registry->get('themesuri', 'horde'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/screen.css'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/admin-dashboard.css'); | ||
| $this->configurator->addThemeStylesheets( |
Contributor
Author
There was a problem hiding this comment.
Fair point, and it applies to all five call sites. As things stand, ^3.2.0 lets Composer pair this with a Core release that lacks the two methods, and these pages would fatal.
@ralflang how do you prefer to handle a base PR that depends on an unreleased Core change? I see two options:
- Raise the constraint to the first Core release that ships Let controllers add the active theme's stylesheets and scripts Core#243, once you know its number — I'd rather not guess a version that doesn't exist yet. Merge order would then be Core#243 first.
- Make this PR independent of the Core version: call the new methods only when
method_exists()says they are there, and fall back to the current/default/stylesheets otherwise. No fatal on any Core version, at the cost of a compatibility branch to remove later.
Happy to push either.
Member
There was a problem hiding this comment.
Don't handle versioning manually. The next base release will depend on a newer version of core.
| $themesUri = $this->registry->get('themesuri', 'horde'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/screen.css'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/settings.css'); | ||
| $this->configurator->addThemeStylesheets( |
Member
There was a problem hiding this comment.
@TDannhauer I suggest you reconfigure copilot to NOT suggest this kind of change. This is handled in our release pipeline.
| $themesUri = $this->registry->get('themesuri', 'horde'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/screen.css'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/settings.css'); | ||
| $this->configurator->addThemeStylesheets( |
| $themesUri = $this->registry->get('themesuri', 'horde'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/screen.css'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/settings.css'); | ||
| $this->configurator->addThemeStylesheets( |
| $themesUri = $this->registry->get('themesuri', 'horde'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/screen.css'); | ||
| $this->assetCollector->addStylesheet($themesUri . '/default/settings.css'); | ||
| $this->configurator->addThemeStylesheets( |
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.
Part of horde/Core#233. Depends on horde/Core#243, which adds the two
methods used here.
Five controllers built their stylesheet URLs by hand with a literal
/default/:Settings/OAuthAccountControllerAdmin/OAuthProviderControllerAdmin/OAuthSystemStatusControllerAdmin/ApiRegistryControllerAdmin/AdminDashboardController(admin-dashboard.css)so these pages always rendered with the default theme whatever the user
selected, and never loaded the theme's
$theme_scripts.Change
Replace the hardcoded
addStylesheet()calls withViewModeConfigurator::addThemeStylesheets(), and calladdThemeScripts()right afterconfigure(). Themes that do not shipsettings.cssoradmin-dashboard.csskeep inheriting the default onesthrough the regular cascade.
Testing
On a FRAMEWORK_6_0 install, with horde/Core#243 and horde/Core#242
applied and a self-contained theme selected: the OAuth account, OAuth
provider and API registry pages render with the user's theme, its scripts
and its display preferences. Classic pages kept rendering normally during
the test. The admin dashboard and OAuth status pages were not opened
during the test; the change there is identical.