Repository navigation
Commands: Add category property to command registration #75612
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,37 @@ | ||
| /** @typedef {import('@wordpress/keycodes').WPKeycodeModifier} WPKeycodeModifier */ | ||
|
|
||
| /** | ||
| * @typedef {'command'|'view'|'edit'|'workflow'|'action'} WPCommandCategory | ||
| */ | ||
|
|
||
| /** | ||
| * Command categories allowed via registerCommand. | ||
| * The 'workflow' category is reserved for internal use | ||
| * and cannot be registered through this API. | ||
| * | ||
| * @type {Set<WPCommandCategory>} | ||
| */ | ||
| const REGISTERABLE_CATEGORIES = new Set( [ | ||
| 'command', | ||
| 'view', | ||
| 'edit', | ||
| 'action', | ||
| ] ); | ||
|
|
||
| /** | ||
| * Configuration of a registered keyboard shortcut. | ||
| * | ||
| * @typedef {Object} WPCommandConfig | ||
| * | ||
| * @property {string} name Command name. | ||
| * @property {string} label Command label. | ||
| * @property {string=} searchLabel Command search label. | ||
| * @property {string=} context Command context. | ||
| * @property {React.JSX.Element} icon Command icon. | ||
| * @property {Function} callback Command callback. | ||
| * @property {boolean} disabled Whether to disable the command. | ||
| * @property {string[]=} keywords Command keywords for search matching. | ||
| * @property {string} name Command name. | ||
| * @property {string} label Command label. | ||
| * @property {string=} searchLabel Command search label. | ||
| * @property {string=} context Command context. | ||
| * @property {WPCommandCategory=} category Command category. | ||
| * @property {React.JSX.Element} icon Command icon. | ||
| * @property {Function} callback Command callback. | ||
| * @property {boolean} disabled Whether to disable the command. | ||
| * @property {string[]=} keywords Command keywords for search matching. | ||
| */ | ||
|
|
||
| /** | ||
|
|
@@ -38,9 +57,17 @@ | |
| * @return {Object} action. | ||
| */ | ||
| export function registerCommand( config ) { | ||
| let { category } = config; | ||
|
|
||
| // Defaults to 'action' if no category is provided or if the category is invalid. Future versions will emit a warning. | ||
| if ( ! category || ! REGISTERABLE_CATEGORIES.has( category ) ) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if there's no category, we shouldn't be showing a warning
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it was by design from @fcoveram that having no category is a mistake. I will move the warning to future versions if you agree.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that we already have a lot of commands in the community like that. I don't want to flood the console with warnings.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I think if I'm not wrong, we're not doing this validation for command loaders (which don't use registercommand if I'm not wrong)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Then we should probably reach a decision for "Action".
Missed that, will do it in the follow up
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what we ended up deciding here, but showing warnings is definitely not ideal. I still believe that every action should belong to a category, even if that one is "uncategorized" |
||
| category = 'action'; | ||
| } | ||
|
|
||
| return { | ||
| type: 'REGISTER_COMMAND', | ||
| ...config, | ||
| category, | ||
| }; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a category like
### EnhancementsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do that in the follow up