Repository navigation
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
0b0d3d4 to
5efd3c4
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Tagging reviewers: @sabernhardt, @t-hamano, @westonruter 🙂 |
t-hamano
left a comment
There was a problem hiding this comment.
Thanks for the PR!
I tested this PR using Core color schemes and plugin's (such as Sleeker Admin Color Schemes) and it works fine!
Does this mean the plugin-defined color scheme is properly applied to the front end? Based on what I checked with the AI, this doesn't seem to be true.
With the current implementation, custom color schemes registered by plugins via wp_admin_css_color() are not applied to the admin bar on the front end. Most plugins register their color schemes on admin_init, so the registered data isn't available on the front end.
Even if we encouraged plugins to register them on init instead, the registered stylesheet targets the entire dashboard and includes generic selectors such as body and a. Loading it as-is on the front end would affect the theme's appearance.
This isn't a regression, since trunk currently uses the default colors on the front end regardless of the selected scheme. However, this PR introduces a discrepancy where core color schemes are applied but plugin color schemes are not, which users who select a custom scheme in the dashboard might perceive as a bug. I don't have a good solution for this at the moment 🤔
| * | ||
| * @since 7.2.0 | ||
| */ | ||
| function wp_admin_bar_add_color_scheme_to_front_end() { |
There was a problem hiding this comment.
| function wp_admin_bar_add_color_scheme_to_front_end() { | |
| function wp_enqueue_admin_bar_color_scheme_styles() { |
I have no strong opinion, but at the very least, we should use 'enqueue' instead of 'add'.
There was a problem hiding this comment.
I think this is better. It's more consistent with the existing wp_enqueue_admin_bar_header_styles() for example. Applied 👍
Sorry, it was bad phrasing from me. I meant when using a plugin-defined scheme, it does not enqueue the CSS as expected (as per the old PR's behavior) 🤦 |
Right, this is an unsolved problem right now. One solution is to allow |
5efd3c4 to
2ed87c0
Compare
fushar
left a comment
There was a problem hiding this comment.
Thanks for the review. Addressed them 👍
I explored this solution. Something like this: Click to opendiff --git i/src/wp-includes/admin-bar.php w/src/wp-includes/admin-bar.php
index c847c8994c..c23fce361e 100644
--- i/src/wp-includes/admin-bar.php
+++ w/src/wp-includes/admin-bar.php
@@ -1491,31 +1491,33 @@ function _get_admin_bar_pref( $context = 'front', $user = 0 ) {
* Enqueues the admin bar color scheme stylesheet on the front end if present.
*
* @since 7.2.0
+ *
+ * @global array $_wp_admin_css_colors
*/
function wp_enqueue_admin_bar_color_scheme_styles() {
+ global $_wp_admin_css_colors;
+
if ( is_admin() ) {
return;
}
+ $registered_color_schemes = $_wp_admin_css_colors ?? array();
+ register_admin_color_schemes();
+ $_wp_admin_css_colors = array_merge( $_wp_admin_css_colors, $registered_color_schemes );
+
$color_scheme = get_user_option( 'admin_color' );
- if ( empty( $color_scheme ) ) {
+ if ( empty( $color_scheme ) || ! isset( $_wp_admin_css_colors[ $color_scheme ] ) ) {
$color_scheme = 'modern';
}
- if ( sanitize_key( $color_scheme ) !== $color_scheme ) {
- return;
- }
-
- $suffix = SCRIPT_DEBUG ? '' : '.min';
- $path = "css/colors/{$color_scheme}/admin-bar{$suffix}.css";
-
- if ( ! file_exists( ABSPATH . 'wp-admin/' . $path ) ) {
+ $admin_bar_url = $_wp_admin_css_colors[ $color_scheme ]->admin_bar_url ?? '';
+ if ( ! $admin_bar_url ) {
return;
}
wp_enqueue_style(
'admin-bar-color-scheme',
- admin_url( $path ),
+ $admin_bar_url,
array( 'admin-bar' )
);
}
diff --git i/src/wp-includes/general-template.php w/src/wp-includes/general-template.php
index 73178b9eca..6c0b24db24 100644
--- i/src/wp-includes/general-template.php
+++ w/src/wp-includes/general-template.php
@@ -5177,16 +5177,39 @@ function paginate_links( $args = '' ) {
* '#07273E', '#14568A', '#D54E21', '#2683AE'
* ) );
*
+ * Alternatively, the third argument can be an array of arguments:
+ *
+ * wp_admin_css_color(
+ * 'classic',
+ * __( 'Classic' ),
+ * array(
+ * 'url' => plugins_url( 'css/colors.css', __FILE__ ),
+ * 'admin_bar_url' => plugins_url( 'css/admin-bar.css', __FILE__ ),
+ * ),
+ * array( '#07273E', '#14568A', '#D54E21', '#2683AE' )
+ * );
+ *
+ * To apply the color scheme to the admin bar on the front end, the color scheme
+ * must be registered on the front end too.
+ *
* @since 2.5.0
+ * @since 7.2.0 The `$url` parameter can be an array of arguments.
*
* @global array $_wp_admin_css_colors
*
- * @param string $key The unique key for this theme.
- * @param string $name The name of the theme.
- * @param string $url The URL of the CSS file containing the color scheme.
- * @param array $colors Optional. An array of CSS color definition strings which are used
- * to give the user a feel for the theme.
- * @param array $icons {
+ * @param string $key The unique key for this theme.
+ * @param string $name The name of the theme.
+ * @param string|array $url {
+ * The URL of the CSS file containing the color scheme, or an array of arguments.
+ *
+ * @type string $url Optional. The URL of the CSS file containing the color scheme.
+ * @type string $admin_bar_url Optional. The URL of the CSS file containing the color scheme
+ * for the admin bar only, loaded on the front end. It should only
+ * contain styles scoped to `#wpadminbar`.
+ * }
+ * @param array $colors Optional. An array of CSS color definition strings which are used
+ * to give the user a feel for the theme.
+ * @param array $icons {
* Optional. CSS color definitions used to color any SVG icons.
*
* @type string $base SVG icon base color.
@@ -5201,11 +5224,20 @@ function wp_admin_css_color( $key, $name, $url, $colors = array(), $icons = arra
$_wp_admin_css_colors = array();
}
+ $args = wp_parse_args(
+ is_array( $url ) ? $url : array( 'url' => $url ),
+ array(
+ 'url' => '',
+ 'admin_bar_url' => '',
+ )
+ );
+
$_wp_admin_css_colors[ $key ] = (object) array(
- 'name' => $name,
- 'url' => $url,
- 'colors' => $colors,
- 'icon_colors' => $icons,
+ 'name' => $name,
+ 'url' => $args['url'],
+ 'admin_bar_url' => $args['admin_bar_url'],
+ 'colors' => $colors,
+ 'icon_colors' => $icons,
);
}
@@ -5220,13 +5252,16 @@ function wp_admin_css_color( $key, $name, $url, $colors = array(), $icons = arra
* @since 3.0.0
*/
function register_admin_color_schemes() {
- $suffix = is_rtl() ? '-rtl' : '';
- $suffix .= SCRIPT_DEBUG ? '' : '.min';
+ $min_suffix = SCRIPT_DEBUG ? '' : '.min';
+ $suffix = ( is_rtl() ? '-rtl' : '' ) . $min_suffix;
wp_admin_css_color(
'modern',
_x( 'Default', 'admin color scheme' ),
- admin_url( "css/colors/modern/colors$suffix.css" ),
+ array(
+ 'url' => admin_url( "css/colors/modern/colors$suffix.css" ),
+ 'admin_bar_url' => admin_url( "css/colors/modern/admin-bar$min_suffix.css" ),
+ ),
array( '#1e1e1e', '#3858e9', '#7b90ff' ),
array(
'base' => '#f3f1f1',
@@ -5250,7 +5285,10 @@ function register_admin_color_schemes() {
wp_admin_css_color(
'light',
_x( 'Light', 'admin color scheme' ),
- admin_url( "css/colors/light/colors$suffix.css" ),
+ array(
+ 'url' => admin_url( "css/colors/light/colors$suffix.css" ),
+ 'admin_bar_url' => admin_url( "css/colors/light/admin-bar$min_suffix.css" ),
+ ),
array( '#e5e5e5', '#6a6a6a', '#c64606', '#007cba' ),
array(
'base' => '#999',
@@ -5262,7 +5300,10 @@ function register_admin_color_schemes() {
wp_admin_css_color(
'blue',
_x( 'Blue', 'admin color scheme' ),
- admin_url( "css/colors/blue/colors$suffix.css" ),
+ array(
+ 'url' => admin_url( "css/colors/blue/colors$suffix.css" ),
+ 'admin_bar_url' => admin_url( "css/colors/blue/admin-bar$min_suffix.css" ),
+ ),
array( '#183751', '#245278', '#437aa8', '#e1a948' ),
array(
'base' => '#e5f8ff',
@@ -5274,7 +5315,10 @@ function register_admin_color_schemes() {
wp_admin_css_color(
'midnight',
_x( 'Midnight', 'admin color scheme' ),
- admin_url( "css/colors/midnight/colors$suffix.css" ),
+ array(
+ 'url' => admin_url( "css/colors/midnight/colors$suffix.css" ),
+ 'admin_bar_url' => admin_url( "css/colors/midnight/admin-bar$min_suffix.css" ),
+ ),
array( '#232a2e', '#333c42', '#69a8bb', '#cf4339' ),
array(
'base' => '#f1f2f3',
@@ -5286,7 +5330,10 @@ function register_admin_color_schemes() {
wp_admin_css_color(
'sunrise',
_x( 'Sunrise', 'admin color scheme' ),
- admin_url( "css/colors/sunrise/colors$suffix.css" ),
+ array(
+ 'url' => admin_url( "css/colors/sunrise/colors$suffix.css" ),
+ 'admin_bar_url' => admin_url( "css/colors/sunrise/admin-bar$min_suffix.css" ),
+ ),
array( '#6f2724', '#8a312d', '#ad631e', '#ccaf0b' ),
array(
'base' => '#f3f1f1',
@@ -5298,7 +5345,10 @@ function register_admin_color_schemes() {
wp_admin_css_color(
'ectoplasm',
_x( 'Ectoplasm', 'admin color scheme' ),
- admin_url( "css/colors/ectoplasm/colors$suffix.css" ),
+ array(
+ 'url' => admin_url( "css/colors/ectoplasm/colors$suffix.css" ),
+ 'admin_bar_url' => admin_url( "css/colors/ectoplasm/admin-bar$min_suffix.css" ),
+ ),
array( '#392751', '#4a3369', '#646c3e', '#d46f15' ),
array(
'base' => '#ece6f6',
@@ -5310,7 +5360,10 @@ function register_admin_color_schemes() {
wp_admin_css_color(
'ocean',
_x( 'Ocean', 'admin color scheme' ),
- admin_url( "css/colors/ocean/colors$suffix.css" ),
+ array(
+ 'url' => admin_url( "css/colors/ocean/colors$suffix.css" ),
+ 'admin_bar_url' => admin_url( "css/colors/ocean/admin-bar$min_suffix.css" ),
+ ),
array( '#2b3f44', '#39535a', '#567958', '#aa9d88' ),
array(
'base' => '#f2fcff',
@@ -5322,7 +5375,10 @@ function register_admin_color_schemes() {
wp_admin_css_color(
'coffee',
_x( 'Coffee', 'admin color scheme' ),
- admin_url( "css/colors/coffee/colors$suffix.css" ),
+ array(
+ 'url' => admin_url( "css/colors/coffee/colors$suffix.css" ),
+ 'admin_bar_url' => admin_url( "css/colors/coffee/admin-bar$min_suffix.css" ),
+ ),
array( '#382e27', '#5c4c40', '#916745', '#9ea476' ),
array(
'base' => '#f3f2f1',Plugins would register the scheme at add_action(
'init',
function () {
wp_admin_css_color(
'demo',
'Demo,
array(
'url' => plugins_url( 'demo-admin-color-scheme/colors.css', __FILE__ ),
'admin_bar_url' => plugins_url( 'demo-admin-color-scheme/admin-bar.css', __FILE__ ),
),
array( '#3b1f4f', '#2c173b', '#e05d9a', '#f2b134' ),
array(
'base' => '#f2eef4',
'focus' => '#ff9ccb',
'current' => '#fff',
)
);
}
);This seems to work in my testing. But I think this should be another ticket. |
| } | ||
|
|
||
| $color_scheme = get_user_option( 'admin_color' ); | ||
| if ( empty( $color_scheme ) ) { |
There was a problem hiding this comment.
| if ( empty( $color_scheme ) ) { | |
| if ( ! is_string( $color_scheme ) ) { |
There was a problem hiding this comment.
I think I'd keep empty() to match with wp-admin's behavior here:
| } | ||
|
|
||
| $suffix = SCRIPT_DEBUG ? '' : '.min'; | ||
| $path = "css/colors/{$color_scheme}/admin-bar{$suffix}.css"; |
There was a problem hiding this comment.
I don't suppose there is any RTL variation, right?
There was a problem hiding this comment.
I believe that's correct. The new CSS (should) only contain color changes, so there shouldn't be any left/right related properties.
fushar
left a comment
There was a problem hiding this comment.
Thanks for the typings comments; applied them and replied to the remaining comments.
t-hamano
left a comment
There was a problem hiding this comment.
@fushar Thanks for the update!
While I think the approach is good, personally, I am quite cautious about moving forward with this PR. This is because it introduces the new wp_enqueue_admin_bar_color_scheme_styles filter callback and the admin-bar-color-scheme style handle. Since @aduth has proposed supporting structured color options in wp_admin_css_color, we might also need to consider the progress on that ticket.
One idea I have is to inject the CSS inline via private methods without adding any public APIs. What do you think? In other words, I'm thinking of making changes like the following to this PR:
diff --git a/src/wp-includes/admin-bar.php b/src/wp-includes/admin-bar.php
index 5a5cada526..5ba5687922 100644
--- a/src/wp-includes/admin-bar.php
+++ b/src/wp-includes/admin-bar.php
@@ -1486,36 +1486,3 @@ function _get_admin_bar_pref( $context = 'front', $user = 0 ) {
return 'true' === $pref;
}
-
-/**
- * Enqueues the admin bar color scheme stylesheet on the front end if present.
- *
- * @since 7.2.0
- */
-function wp_enqueue_admin_bar_color_scheme_styles(): void {
- if ( is_admin() ) {
- return;
- }
-
- $color_scheme = get_user_option( 'admin_color' );
- if ( empty( $color_scheme ) ) {
- $color_scheme = 'modern';
- }
-
- if ( sanitize_key( $color_scheme ) !== $color_scheme ) {
- return;
- }
-
- $suffix = SCRIPT_DEBUG ? '' : '.min';
- $path = "css/colors/{$color_scheme}/admin-bar{$suffix}.css";
-
- if ( ! file_exists( ABSPATH . 'wp-admin/' . $path ) ) {
- return;
- }
-
- wp_enqueue_style(
- 'admin-bar-color-scheme',
- admin_url( $path ),
- array( 'admin-bar' )
- );
-}
diff --git a/src/wp-includes/class-wp-admin-bar.php b/src/wp-includes/class-wp-admin-bar.php
index 200d915200..2f8ac2bc60 100644
--- a/src/wp-includes/class-wp-admin-bar.php
+++ b/src/wp-includes/class-wp-admin-bar.php
@@ -72,6 +72,10 @@ class WP_Admin_Bar {
wp_enqueue_script( 'admin-bar' );
wp_enqueue_style( 'admin-bar' );
+ if ( ! is_admin() ) {
+ $this->add_color_scheme_styles();
+ }
+
/**
* Fires after WP_Admin_Bar is initialized.
*
@@ -80,6 +84,39 @@ class WP_Admin_Bar {
do_action( 'admin_bar_init' );
}
+ /**
+ * Applies the current user's admin color scheme to the admin bar on the front end.
+ *
+ * Inlines the color scheme's admin-bar.css, built from wp-admin/css/colors/.
+ * In the admin, the color scheme stylesheet already styles the admin bar.
+ *
+ * @since 7.2.0
+ */
+ private function add_color_scheme_styles() {
+ $color_scheme = get_user_option( 'admin_color' );
+
+ if ( empty( $color_scheme ) ) {
+ $color_scheme = 'modern';
+ }
+
+ if ( ! is_string( $color_scheme ) || sanitize_key( $color_scheme ) !== $color_scheme ) {
+ return;
+ }
+
+ $suffix = SCRIPT_DEBUG ? '' : '.min';
+ $path = ABSPATH . "wp-admin/css/colors/{$color_scheme}/admin-bar{$suffix}.css";
+
+ if ( ! is_readable( $path ) ) {
+ return;
+ }
+
+ $css = file_get_contents( $path );
+
+ if ( $css ) {
+ wp_add_inline_style( 'admin-bar', $css );
+ }
+ }
+
/**
* Adds a node (menu item) to the admin bar menu.
*
diff --git a/src/wp-includes/default-filters.php b/src/wp-includes/default-filters.php
index 40b658e14b..025a371781 100644
--- a/src/wp-includes/default-filters.php
+++ b/src/wp-includes/default-filters.php
@@ -726,7 +726,6 @@ add_action( 'activate_header', '_wp_admin_bar_init' );
add_action( 'wp_body_open', 'wp_admin_bar_render', 0 );
add_action( 'wp_footer', 'wp_admin_bar_render', 1000 ); // Back-compat for themes not using `wp_body_open`.
add_action( 'in_admin_header', 'wp_admin_bar_render', 0 );
-add_action( 'admin_bar_init', 'wp_enqueue_admin_bar_color_scheme_styles' );
// Former admin filters that can also be hooked on the front end.
add_action( 'media_buttons', 'media_buttons' );|
I considered inline styles as well during my exploration. I think that's the most future-proof solution, considering that |
We might actually need to benchmark and compare these approaches. My guess is that the performance impact will be negligible. For reference, |
|
There doesn't seem to be much difference, even though this report was generated by AI. Setup
mu-plugin used for the A/B switch<?php
add_action(
'admin_bar_init',
static function () {
$styles = wp_styles();
if ( ! isset( $styles->registered['admin-bar-color-scheme'], $_GET['ab'] ) ) {
return;
}
$style = $styles->registered['admin-bar-color-scheme'];
$style->src = str_replace( 'admin-bar.css', 'admin-bar.min.css', $style->src );
if ( 'inline' === $_GET['ab'] ) {
$rel = substr( $style->src, strlen( admin_url() ) );
wp_style_add_data( 'admin-bar-color-scheme', 'path', ABSPATH . 'wp-admin/' . $rel );
}
},
11
);ResultsMedian ± median absolute deviation (ms). Diff = inline − file.
With inlining, the HTML grows by about 4 KB on every page view (uncompressed in this local environment). Takeaways
Caveats
|
|
Thanks for the test @t-hamano, I'm contemplating and brainstorming another solution that combines both approaches. Give me a few hours... |
|
All right, I have this other approach here: #14011 which inlines CSS variables instead. My vision is to make the solution future-proof with https://core.trac.wordpress.org/ticket/65776 and align with the current "WPDS-fication" of classic PHP admin screens. I asked a question for @aduth in that ticket. If we want to make |
Trac ticket: https://core.trac.wordpress.org/ticket/64762
Follow-up to: #11354 by @sabernhardt.
This PR enqueues a small CSS in the logged-in frontend pages so that the admin bar is shown using the current admin bar color scheme.
The improvements over the linked PR are as follows.
fresh), this PR simply checks for the existence of the newly builtadmin-barCSS file usingfile_exists().colors.scssandadmin-bar.scssby extracting the color definition into a shared_scheme.scsspartial.I also tried to resolve all review comments from the original PR.
I tested this PR using Core color schemes and plugin's (such as Sleeker Admin Color Schemes) and it works fine!
Use of AI Tools
Used Claude Opus 5.5 to rebase the original PR and brainstorm solutions for improvements.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.