Zoom out: try pattern inserter instead of starter pattern modal #61489
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
[Type] Experimental
|
Size Change: -13 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
MaggieCabrera
force-pushed
the
try-pattern-inserter-2
branch
from
b0f5bb9
to
7024e02
Compare
ellatrix
force-pushed
the
try/starter-pattern-inserter
branch
2 times, most recently
from
fd4fcda
to
5d9f316
Compare
scruffian
force-pushed
the
try/starter-pattern-inserter
branch
from
34379ec
to
299e7d4
Compare
|
Something isn't working quite right on this PR when I test with playground: testing.zoom.out.inserter.movOpens to the Inserter but I can't insert any patterns. That's because the focus isn't on the post content section :D What's the plan for starter patterns here? I think I would be more into this PR if we surfaced starter patterns still and dropped folks into the Editor. Speaking personally, I find it really annoying to have to close out of the starter patterns nearly every time I create a page (usually for testing purposes but this is true when I'm generally creating content on my sites) and this would work around that nicely while still surfacing patterns. |
|
The rebase probably broke it |
|
Quick question -- will this open to a dedicated set of patterns with a category set to starter patterns? I think that's our best bet here rather than just opening up to all patterns as it seems to based on the GIF above in the PR description. |
|
That's exactly what the PR does, the GIF is old. I haven't had the time to update it, this is the last thing we did obviously Friday and I've been working non stop on other stuff sorry |
ellatrix
force-pushed
the
try/starter-pattern-inserter
branch
from
299e7d4
to
c1356cd
Compare
|
Fixed the site editor, and updated the GIF |
ellatrix
commented
May 16, 2024
| return sectionRootClientId; | ||
| } | ||
| } | ||
| return getBlockRootClientId(); |
There was a problem hiding this comment.
This is a useless call without passing a selected block, it always returns ''.
ellatrix
commented
May 16, 2024
| const { get } = select( preferencesStore ); | ||
| const getBlockSectionRootClientId = () => { | ||
| if ( __unstableGetEditorMode() === 'zoom-out' ) { |
There was a problem hiding this comment.
We should always fall back to the section ID without checking zoom-out mode. Otherwise the inserter won't have any patterns, and it's impossible to enable zoom-out mode. It's not a bad thing to always fall back to section ID regardless of mode.
Completely agree, I personally also find that quite annoying. With #61518 it will be even better, because it will only take up one sidebar. I think a remaining thing to do for this PR is allowing escape from zoom-out when you click the canvas. |
|
I still can't get the PR to work well on playground for testing, particularly within the Site Editor. A quick clip for awareness: GB.test.movIn any case, the drawbacks I see in this approach are:
I do think #61518 will help with nudging folks but the previews are going to be pretty limited. Still worth trying and, to me, with the tradeoff compared to needing to x out of the modal every single time. |
|
How do I test this in the https://playground.wordpress.net/ ? In general it would be better to not have something take over the full screen when creating a new page. Having something that gives a hint about starter patterns would be better. As one could quickly choose to open patterns and check for starter content. Most of the time I go with blank pages. Having to close something every time a new page is created becomes counter protective. So perhaps as one opens a new page the left Block Inserter could be open and Starter Content if there is one could be highlighted. As is mentioned in this comment by Ella: |
|
All the animation that occurs on load is a bit distracting, is there any way to skip it? Should we force-close the Inspector in this case? If the idea is to funnel folks into choosing patterns then it's also quite distracting. Initial focus is a bit unexpected to me, should it be the first item in the list of patterns rather than the Patterns tab itself? |
5 tasks
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?
Instead of showing the starter patterns modal when creating a draft page, this PR tries opening the patterns inserter instead (+ zoom out mode).
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast