Preference modal: avoid fetching all reusable blocks when the site editor loads #66621
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ellatrix
added
the
[Type] Regression
|
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: +2 B (0%) Total Size: 1.81 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 74571e7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11603064228
|
It was a compromise I guess. My opinion, stated several times, was that the modal is annoying and it should be removed completely until something better is proposed. I'm still of that opinion. |
Good question. I guess because the modal itself will only display if the site has start patterns, so it could be confusing if there's a preference that doesn't appear to do anything? I.e. should this preference be visible on Classic themes?
I agree. If done well, I don't mind the idea of a modal for selecting patterns, like via the "Explore all patterns" button in the patterns tab in the inserter. To me this particular modal feels more intrusive than useful. In terms of the scope of this PR, nice idea moving to a wrapper and returning early to skip calling the hooks when not needed! Looks like some of the tests are failing. Are they related to the change?
If you have ideas about how it could be less annoying, let's open an issue for it. |
ramonjd
changed the title
|
Tests need to be fixed. I updated the test instructions to make it clearer what this PR does. |
ramonjd
changed the title
|
Thanks for getting the PR up. LGTM. I'll fix the failing tests and merge it. |
ramonjd
changed the title
ramonjd
approved these changes
Oct 30, 2024
|
Thanks for getting this over the line, @ramonjd! 🙇 |
ramonjd
added
the
Backport to WP 6.7 Beta/RC
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
gutenbergplugin
pushed a commit
that referenced
this pull request
Oct 31, 2024
…ce modal (#66621) Avoid fetching block patterns when the site editor loads. Move useStartPatterns() hook to a wrapper and return early if the preferences modal is not active. Fix JS unit tests --------- Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: bbee6ee |
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
…ce modal (WordPress#66621) Avoid fetching block patterns when the site editor loads. Move useStartPatterns() hook to a wrapper and return early if the preferences modal is not active. Fix JS unit tests --------- Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Introduced in #65026, which adds an option to the preferences modal to hide the starter patterns modal. This is causing all reusable blocks and patterns to be fetched on load.
We can prevent this by only calling the hook when the modal is open.
But, why do we need to load all starter patterns in the first place? Why not just always show the option?
And, why do we need this option at all? What happened to: decisions, not options? This modal should be adjusted to be less annoying in the first place.
Why?
All reusable blocks and patterns are fetched on editor load.
How?
Moving the
useStartPatternshook into a wrapper and returning early in the parent component.Testing Instructions
/wp/v2/block-patterns/patterns&_locale=useris not called.Testing Instructions for Keyboard
Screenshots or screencast