Toolbar color schemes on front end without global variable#11354
Toolbar color schemes on front end without global variable#11354sabernhardt wants to merge 12 commits intoWordPress:trunkfrom
Conversation
…chemes that uses the extracted admin toolbar styles.
Note to self: Do not attempt this at 3AM.
|
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. |
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. |
| if ( ! $admin_bar_url ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Is this here in case someone filters admin_url to return an empty string for some reason?
| #wpadminbar:not(.mobile) .ab-top-menu > #wp-admin-bar-recovery-mode > .ab-item:focus { | ||
| color: variables.$adminbar-recovery-exit-text; | ||
| background-color: variables.$adminbar-recovery-exit-background-alt; | ||
| } |
There was a problem hiding this comment.
The recovery mode CSS is removed here but isn't added to src/wp-admin/css/colors/_admin-bar.scss. Is that intentional?
(Props Claude.)
|
|
||
| global $_wp_admin_css_colors; | ||
| $color_scheme = get_user_option( 'admin_color' ); | ||
| $core_list = array( 'modern', 'light', 'blue', 'coffee', 'ectoplasm', 'midnight', 'ocean', 'sunrise' ); |
There was a problem hiding this comment.
| $core_list = array( 'modern', 'light', 'blue', 'coffee', 'ectoplasm', 'midnight', 'ocean', 'sunrise' ); | |
| /** Refer to {@see register_admin_color_schemes()} for this list, omitting 'fresh'. */ | |
| $core_list = array( 'modern', 'light', 'blue', 'coffee', 'ectoplasm', 'midnight', 'ocean', 'sunrise' ); |
| @@ -0,0 +1,140 @@ | |||
| @forward 'variables' show $scheme-name, $base-color, $body-background, $button-color, $custom-welcome-panel, $dashboard-accent-1, $dashboard-accent-2, $dashboard-icon-background, $form-checked, $highlight-color, $icon-color, $link, $link-focus, $low-contrast-theme, $menu-bubble-text, $menu-collapse-focus-icon, $menu-collapse-text, $menu-highlight-background, $menu-highlight-icon, $menu-highlight-text, $menu-submenu-text, $menu-submenu-focus-text, $menu-submenu-background, $notification-color, $text-color; | |||
There was a problem hiding this comment.
According to Claude, this list is overly broad. It can be reduced to be more truthful about what is being used.
| @forward 'variables' show $scheme-name, $base-color, $body-background, $button-color, $custom-welcome-panel, $dashboard-accent-1, $dashboard-accent-2, $dashboard-icon-background, $form-checked, $highlight-color, $icon-color, $link, $link-focus, $low-contrast-theme, $menu-bubble-text, $menu-collapse-focus-icon, $menu-collapse-text, $menu-highlight-background, $menu-highlight-icon, $menu-highlight-text, $menu-submenu-text, $menu-submenu-focus-text, $menu-submenu-background, $notification-color, $text-color; | |
| @forward 'variables' show $base-color, $icon-color, $highlight-color, $text-color, $menu-submenu-text, $menu-submenu-focus-text, $menu-submenu-background; |
(Props Claude.)
| if ( is_admin() ) { | ||
| $this->assertFalse( wp_style_loader_src( '', 'colors' ) ); | ||
| } else { | ||
| $this->assertStringContainsString( '/colors.css', wp_style_loader_src( '', 'colors' ) ); | ||
| } | ||
| $this->assertFalse( wp_style_loader_src( '', 'colors' ) ); |
There was a problem hiding this comment.
Ideally there'd be two tests, one which sets the global state so that is_admin() returns true and another test where is_admin() is false. This would ensure the code in wp_admin_bar_add_color_scheme_to_front_end() is actually tested.
Alternative option to #11267 that uses the same Sass and built stylesheets but avoids using the
$_wp_admin_css_colorsglobal variable.Trac 64762
Use of AI Tools
@johnbillion shared these details:
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.