-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix post editor's API for pinning plugin items #34155
Conversation
Size Change: +14 B (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
I think those methods exist only for backward compatibility as they were extracted to |
}; | ||
export function* togglePinnedPluginItem( pluginName ) { | ||
const isPinned = yield controls.select( | ||
interfaceStore.name, |
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.
Unrelated comment: did we miss controls when introducing the way to use store objects as a reference to select
and dispatch
calls?
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.
I haven't seen any that use a string literal, so I feel like this was covered. Just checked and the linting also works 👍
I did wonder whether interface's scope could also be using the store name.
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.
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.
I haven't tested but the proposed changes make sense 👍🏻
Thanks! |
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.
I think those methods exist only for backward compatibility as they were extracted to @wordpress/interface from @wordpress/edit-post. @jorgefilipecosta, do you remember how it should work?
Yes, these methods exist just for backward compatibility, all the logic is on the interface package. During the refactor we probably made a mistake, thank you for fixing this @talldan!
Description
Something I noticed in passing, the
isPluginItemPinned
selector andtogglePinnedPluginItem
actions in the post editor are broken. The selector has the wrong params, and the action has no matching reducer so doesn't do anything.These APIs not used internally by any code, so they're really just kept as public APIs. It could also be an option to just delete them, though if a plugin is using them (https://www.wpdirectory.net/search/01FDE8F27RZMBXZWB15C75Q5JB) that would cause an error to be triggered.
How has this been tested?
wp.data.select( 'core/edit-post' ).isPluginItemPinned( 'myAwesomePlugin' );
, it should return the defaulttrue
value first timewp.data.dispatch( 'core/edit-post' ).togglePinnedPluginItem( 'myAwesomePlugin' );
to toggle it off.wp.data.select( 'core/edit-post' ).isPluginItemPinned( 'myAwesomePlugin' );
, it should returnfalse
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.js
files for terms that need renaming or removal).