Advertise the command palette in the admin bar#10985
Advertise the command palette in the admin bar#10985brandonpayton wants to merge 1 commit intoWordPress:trunkfrom
Conversation
|
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. |
| wp_add_inline_script( | ||
| 'admin-bar', | ||
| '( function() { | ||
| var isMac = window.navigator.platform.indexOf( "Mac" ) !== -1; |
There was a problem hiding this comment.
| var isMac = window.navigator.platform.indexOf( "Mac" ) !== -1; | |
| var isMac = window.navigator.platform ? window.navigator.platform.indexOf( 'Mac' ) !== -1 : false; |
For consistency. Core use these check for isMac.
There was a problem hiding this comment.
Maybe this in more modern and readable?
| var isMac = window.navigator.platform.indexOf( "Mac" ) !== -1; | |
| const isMac = window.navigator.platform?.includes('Mac') ?? false |
There was a problem hiding this comment.
Note that PhpStorm is flagging platform as being deprecated:
Apparently we should use navigator.userAgentData.platform now: https://medium.com/@jortiz.dev/bye-navigator-platform-here-is-the-alternative-939b883bf050
But it isn't available everywhere: https://caniuse.com/mdn-api_navigator_useragentdata
The article suggets: navigator.userAgent.includes('Mac')
| * @param WP_Admin_Bar $wp_admin_bar The WP_Admin_Bar instance. | ||
| */ | ||
| function wp_admin_bar_command_palette_menu( $wp_admin_bar ) { | ||
| if ( ! is_admin() ) { |
There was a problem hiding this comment.
Eventually it would be nice if the the command palette was on the frontend as well.
| * | ||
| * @param WP_Admin_Bar $wp_admin_bar The WP_Admin_Bar instance. | ||
| */ | ||
| function wp_admin_bar_command_palette_menu( $wp_admin_bar ) { |
There was a problem hiding this comment.
| function wp_admin_bar_command_palette_menu( $wp_admin_bar ) { | |
| function wp_admin_bar_command_palette_menu( WP_Admin_Bar $wp_admin_bar ): void { |
| wp_add_inline_script( | ||
| 'admin-bar', | ||
| '( function() { | ||
| var isMac = window.navigator.platform.indexOf( "Mac" ) !== -1; |
There was a problem hiding this comment.
Note that PhpStorm is flagging platform as being deprecated:
Apparently we should use navigator.userAgentData.platform now: https://medium.com/@jortiz.dev/bye-navigator-platform-here-is-the-alternative-939b883bf050
But it isn't available everywhere: https://caniuse.com/mdn-api_navigator_useragentdata
The article suggets: navigator.userAgent.includes('Mac')
| $wp_admin_bar->add_node( | ||
| array( | ||
| 'id' => 'command-palette', | ||
| 'title' => '<span class="ab-label" aria-hidden="true"></span>' . |
There was a problem hiding this comment.
This can use kbd and I don't think it should have aria-hidden because then screen readers won't be able to know what the command is to open the palette.
| 'title' => '<span class="ab-label" aria-hidden="true"></span>' . | |
| 'title' => '<kbd class="ab-label"></kbd>' . |
| '( function() { | ||
| var isMac = window.navigator.platform.indexOf( "Mac" ) !== -1; | ||
| var label = document.querySelector( "#wp-admin-bar-command-palette .ab-label" ); | ||
| if ( label ) { | ||
| label.textContent = isMac ? "\u2318K" : "Ctrl+K"; | ||
| } | ||
| } )();' |
There was a problem hiding this comment.
Depending on how we determine isMac, we can eliminate adding isMac variable and we can also use a nowdoc, and factor out the "+K":
| '( function() { | |
| var isMac = window.navigator.platform.indexOf( "Mac" ) !== -1; | |
| var label = document.querySelector( "#wp-admin-bar-command-palette .ab-label" ); | |
| if ( label ) { | |
| label.textContent = isMac ? "\u2318K" : "Ctrl+K"; | |
| } | |
| } )();' | |
| <<<'JS' | |
| ( () => { | |
| const label = document.querySelector( "#wp-admin-bar-command-palette .ab-label" ); | |
| if ( label ) { | |
| label.textContent = ( navigator.userAgent.includes("Mac") ? "\u2318" : "Ctrl" ) + '+K'; | |
| } | |
| } )(); | |
| JS |
The indentation of the JS on the last line is “new” in PHP 7.4.
The use of JS for the heredoc/nowdoc causes IDEs to do syntax highlighting:
| ) | ||
| ); | ||
|
|
||
| wp_add_inline_script( |
There was a problem hiding this comment.
This should also implement a click event handler on the link for the admin bar item which prevents default but then also opens the command palette. In the post editor, clicking on the shortcut also causes the palette to open.
The admin bar item has a link so if a user is tabbing through items with the keyboard and they hit enter on this item, at the moment it does nothing. It should open the command palette.
There was a problem hiding this comment.
WordPress/gutenberg#75757 is designed to open the command palette (and prevent the click default).
An element that does nothing should not be a link (no href).
|
Thank you, everyone, for your feedback on this. Let's close this in favor of WordPress/gutenberg#75757 which appears to be better and getting more focus from its author. |
|
@brandonpayton, WordPress/gutenberg#75757 is required for the Gutenberg plugin and client-side processing, but it is not automatically included in WordPress core. Therefore, PHP code, i.e. server-side processing, basically requires submitting patches directly to WordPress core. It looks like this PR can't be reopened so I'll submit a new one, but thank you for your efforts! |
Thank you, @t-hamano! |
This PR adds a button to invoke the command palette from the admin bar. The button label renders the symbolic "Command K" for Mac and "Ctrl+K" on all other platforms.
@annezazu mentioned the possibility of using AI and Playground for addressing this ticket, and we gave it a shot with Claude. It took some correction, but the result seems simple enough. I'm not sure whether the hotkey text should be translatable but guess it does not.
Trac ticket: https://core.trac.wordpress.org/ticket/64672
Use of AI Tools
This patch was generated using Claude Code with a few manual changes.
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.