Closed ellatrix closed 2 days ago
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 props-bot
label.
If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
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
Filename | Size | Change |
---|---|---|
build/editor/index.min.js |
106 kB | +2 B (0%) |
Flaky tests detected in 74571e77655984edf77cf65bf5aa3418f634629e. Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.
đ Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11603064228 đ Reported issues:
/test/e2e/specs/editor/various/taxonomies.spec.js
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.
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.
But, why do we need to load all starter patterns in the first place? Why not just always show the option?
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?
My opinion, https://github.com/WordPress/gutenberg/pull/65026#issuecomment-2333723060, was that the modal is annoying and it should be removed completely until something better is proposed.
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?
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.
If you have ideas about how it could be less annoying, let's open an issue for it.
Tests need to be fixed.
I updated the test instructions to make it clearer what this PR does.
Thanks for getting the PR up. LGTM.
I'll fix the failing tests and merge it.
Thanks for getting this over the line, @ramonjd! đ
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: bbee6eed09f70021fb751b8b5c87a55ec66f6617
@ramonjd Yes, in my opinion too, this modal is annoying to ALL users, and should just be removed we should trigger zoom out mode as proposed in #61489. :)
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
useStartPatterns
hook into a wrapper and returning early in the parent component.Testing Instructions
/wp/v2/block-patterns/patterns&_locale=user
is not called.Testing Instructions for Keyboard
Screenshots or screencast