-
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
Block Bindings: Accept client ID as parameter for useBlockBindingsUtils
#65818
Block Bindings: Accept client ID as parameter for useBlockBindingsUtils
#65818
Conversation
Size Change: +1 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
||
let clientId; | ||
beforeEach( async () => { | ||
const block = createBlock( 'core/paragraph', { |
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.
Super nuanced nit. I believe that you can use here dispatch( blockEditorStore ).resetBlocks( [ block ] );
instead of insertBlocks
.
This way dispatch( blockEditorStore ).resetBlocks( [] );
could be run only once in afterAll
instead of afterEach
.
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.
That's a good one 🙂 If I understood you correctly, it has been addressed in this commit: e52bab2
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.
Yes, exactly that 👍🏻
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.
That's a nice addition. I'm not sure why it's still a draft. To me it's good to go.
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. |
Because I didn't have time to properly review it after pushing and then I forgot to mark it as ready 😄 I have just done that and addressed the comment. |
Flaky tests detected in e52bab2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11256712589
|
There was a conflict while trying to cherry-pick the commit to the wp/6.7 branch. Please resolve the conflict manually and create a PR to the wp/6.7 branch. PRs to wp/6.7 are similar to PRs to trunk, but you should base your PR on the wp/6.7 branch instead of trunk.
|
…ils` (#65818) * Pass `clientId` to `useBlockBindingsUtils` * Add unit tests for `useBlockBindingsUtils` * Use `resetBlocks` instead of `insertBlocks` in unit test Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
…ils` (#65818) * Pass `clientId` to `useBlockBindingsUtils` * Add unit tests for `useBlockBindingsUtils` * Use `resetBlocks` instead of `insertBlocks` in unit test Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
…ils` (#65818) * Pass `clientId` to `useBlockBindingsUtils` * Add unit tests for `useBlockBindingsUtils` * Use `resetBlocks` instead of `insertBlocks` in unit test Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
…ils` (WordPress#65818) * Pass `clientId` to `useBlockBindingsUtils` * Add unit tests for `useBlockBindingsUtils` * Use `resetBlocks` instead of `insertBlocks` in unit test Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
What?
As discussed here, I'm adding the possibility of passing a
clientId
touseBlockBindingsUtils
instead of always relying on the edit context.Additionally, I added a few unit tests to cover the different use cases.
Why?
It is a common pattern in other APIs and it is missing in this one.
How?
I added some logic to rely on the parameter and, if it isn't provided, use the client ID returned by
useBlockEditContext
.Apart from that, I added unit tests.
Testing Instructions
It is a bit hard to test this manually because code needs to run inside a component or a hook. I added unit tests to cover the different use cases. Additionally, post meta and pattern overrides are using these utils and are covered with e2e tests.