-
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
[Pattern Overrides] Only set block editing mode on first load #59229
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +79 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
6f53cc9
to
a54ba6c
Compare
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. |
Flaky tests detected in a54ba6c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8013753070
|
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.
Thanks for the focus on performance @kevin940726 👍
The changes make sense to me but I think after quite a while away from pattern overrides I'll need more detailed test instructions to make sure I'm adequately smoke testing this vs trunk.
Any chance you could flesh out those test instructions a little more, then I can help get this one approved? 🙏
Oops, yeah, I could provide some more context there. This PR doesn't change functionality so the pattern overrides feature should stay the same. I don't have a specific testing structure for this PR, other than what the e2e tests already covered. I guess the manual instructions in #53705 (comment) still apply here though! 🙏 |
Given that RC1 is today, I'm removing this from 6.5 as non essential. |
What?
A small performance improvement for pattern overrides.
Why?
The original code calls
setBlockEditingMode
multiple times whenever the pattern updates. It'll trigger the subscribers forblock-editor
multiple times which can be a problem later on for large patterns.How?
Since synced patterns are "locked", we only have to set the editing mode once on the first load (and whenever the editing mode of the pattern itself changes.)
This could be improved when we allow adding blocks in the patterns though.
Testing Instructions
Everything should work correctly as on trunk and CI should pass.
Screenshots or screencast
N/A