Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #4439: Denote which theme has been set as the admin theme #3166

Open
wants to merge 1 commit into
base: 1.x
Choose a base branch
from
Open

Issue #4439: Denote which theme has been set as the admin theme #3166

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion core/modules/system/system.admin.inc
View file
Open in desktop
Expand Up @@ -111,7 +111,10 @@ function system_themes_page() {
$themes = system_rebuild_theme_data();
uasort($themes, 'system_sort_modules_by_info_name');

$theme_default = config_get('system.core', 'theme_default');
$config = config('system.core');
$theme_default = $config->get('theme_default');
$admin_theme = $config->get('admin_theme');

$theme_groups = array(
'enabled' => array(),
'disabled' => array(),
Expand All @@ -123,6 +126,7 @@ function system_themes_page() {
}
$admin_theme_options[$theme->name] = $theme->info['name'];
$theme->is_default = ($theme->name == $theme_default);
$theme->is_admin = ($theme->name == $admin_theme);
Comment on lines 128 to +129
Copy link
Member Author

@klonos klonos Jun 11, 2020 •
edited

Choose a reason for hiding this comment

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

The way this is implemented these properties are available only within the context of the themes listing page. I would like us to move these into system_rebuild_theme_data() and/or list_themes(), so that they become available info on themes in general. Would this be an API change?

Copy link
Member

Choose a reason for hiding this comment

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

I think moving it into list_themes() would be appropriate. It would be an API expansion to do so, since these properties would be made available in more places than they had been previously. That's fine to do in our 1.x release. In our docblock for list_themes() we'd just have to include documentation on the new properties and include @since sections for the new properties.

Then instead of this function (system_themes_page()) calling $themes = system_rebuild_theme_data();, it should pull from list_themes() instead so it doesn't have to duplicate the work.


// Identify theme screenshot.
$theme->screenshot = NULL;
Expand Down Expand Up @@ -204,6 +208,13 @@ function system_themes_page() {
$theme->notes[] = t('default theme');
}

// When the admin theme is set to be the same as the default theme, then
// admin_theme in system.core.json is set to the string "0".
if ($theme->is_admin || ($admin_theme == FALSE && $theme->is_default)) {
$theme->classes[] = 'theme-admin';
$theme->notes[] = t('administration theme');
}

// Sort enabled and disabled themes into their own groups.
$theme_groups[$theme->status ? 'enabled' : 'disabled'][] = $theme;
}
Expand Down