-
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
Try nesting registry to patch setAttributes
#55289
Try nesting registry to patch setAttributes
#55289
Conversation
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/block-supports/pattern.php |
Size Change: -516 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
Flaky tests detected in 108a7a2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6490577874
|
const subRegistry = useMemo( () => { | ||
return { | ||
...registry, | ||
dispatch( store ) { | ||
if ( | ||
store !== blockEditorStore && | ||
store !== blockEditorStore.name | ||
) { | ||
return registry.dispatch( store ); | ||
} | ||
const dispatch = registry.dispatch( store ); | ||
const select = registry.select( store ); | ||
return { | ||
...dispatch, | ||
updateBlockAttributes( | ||
clientId, | ||
attributes, | ||
uniqueByBlock | ||
) { | ||
return updateBlockAttributes( patternClientId )( | ||
clientId, | ||
attributes, | ||
uniqueByBlock | ||
)( { | ||
registry, | ||
select, | ||
dispatch, | ||
} ); | ||
}, | ||
}; | ||
}, | ||
}; | ||
}, [ registry, patternClientId ] ); |
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.
@gziolo This is another alternative to #54233, which nests registry
to monkey-patch updateBlockAttributes
for only a given sub-tree. This only solves the "dispatching" side of things too though, for selecting it's still getting the attributes from the HOC. Just want to get a quick check from you or anyone on what you think of this approach? Personally, even though it's still monkey-patching which could seem a little bit hacky, I think this is still better than patching updateBlockAttributes
directly.
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.
Thank you for sharing it. That’s a pretty interesting idea because it would potentially limit the impact of adding additional logic to updateBlockAttributes
. If paired with the selector that returns the list of attributes for the block, we would have quite a robust solution. Have you measured the performance impact on typing in general when applying changes to updateBlockAttributes
in the original block editor store in the previous prototype? I’m curious what’s the rationale behind using the nested registry. One note, I see there is useEntityBlockEditor
used in the block so there might be already a nested registry present.
This is superseded by #56235. |
What?
Alternative to #53887 and #54233. Try nesting registry to override
updateBlockAttributes
for a given tree.Why?
Override
updateBlockAttributes
directly feels out-of-place and unnecessary for regular updates.How?
Monkey-patching
setBlockAttributes
and create a sub-registry.Testing Instructions
Follow the same instructions in #53887.
Screenshots or screencast
TBD