-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add selector for getting section root clientId #65001
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +434 B (+0.02%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
I'll rebase this so we can normalize towards a single selector. |
0482bee
to
1392caf
Compare
@@ -494,6 +492,8 @@ export default function useBlockDropZone( { | |||
getBlockNamesByClientId, | |||
getDraggedBlockClientIds, | |||
getBlockType, | |||
getSectionRootClientId, | |||
isZoomOutMode, |
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 isZoomOutMode
dep isn't new, just the getSectionRootClientId
above.
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.
Nice change, this saves from having to pass the key around everywhere.
Tests well for me and code looks good. 👍
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 is much neater, thanks!
export function getSectionRootClientId( state ) { | ||
return state.settings?.[ sectionRootClientIdKey ]; | ||
} |
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.
Having a JSDoc block could be helpful in the future.
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.
Yeh we really need to do that.
What?
Normalizes access to
sectionRootClientId
to a single private selector.Why?
Designed as a "code quality" fix. We have two options for improving the privacy of this property:
No matter which option we go with we'll want to normalize access to a single selector. This PR does that.
This PR is designed to be merged after one of the above PRs has been merged.
How?
Creates a single private selector to access the
sectionRootClientId
.Testing Instructions
Same as #65000. Basically "does Zoom Out mode still work" as per
trunk
?Testing Instructions for Keyboard
Screenshots or screencast