-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Patterns: Allow inserting of unsynced patterns from quick inserter #52866
Conversation
Ran out of time to get this properly tested and to sort the components changelog, etc. will get it finalised for review tomorrow. |
Size Change: +340 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
Flaky tests detected in e2d4f46. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5652214150
|
@@ -505,9 +505,14 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { | |||
) { | |||
__unstableMarkLastChangeAsPersistent(); | |||
} | |||
//Unsynced patterns are nested in an array so we need to flatten them. |
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.
I was initially flattening this in the autocomplete component, but decided there were more potential backwards compat issues modifying that public API.
It seems like this replaceBlocks
API is currently only ever expecting an array with block objects rather than an array with a nested array of blocks, so this seems like a safe extension, but open to thoughts about whether this is the best approach.
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.
Nice work @glendaviesnz 👍
This definitely helps clear up the confusion around patterns via the quick inserter.
✅ Both unsynced and synced patterns can be inserted by name via the quick inserter
✅ Only synced patterns appear under the synced patterns tab
❓ Not all my unsynced patterns were displayed under the My Patterns category in the Patterns inserter tab. No console errors are present either. Is this a known issue?
Patterns | Inserter Tab |
---|---|
![]() |
![]() |
All my unsynced patterns appear correctly in the quick inserter though.
![Screenshot 2023-07-25 at 11 54 41 am](https://private-user-images.githubusercontent.com/60436221/255765667-847c2c42-c16f-46f0-8c10-dd089f1615b7.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5MTA4ODMsIm5iZiI6MTczODkxMDU4MywicGF0aCI6Ii82MDQzNjIyMS8yNTU3NjU2NjctODQ3YzJjNDItYzE2Zi00NmYwLThjMTAtZGQwODlmMTYxNWI3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDA2NDMwM1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWYwMTIxY2Q4NzgwMjJkZWU2NjM2Y2JmZGJlZGM3MTJhMDVhNTY5YWQyMmQzZjFmODA4MDcxYmViYjVmMjUyMWUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.O7vD91CIcQG1zzN81EzYSoE15TqX7sB25u1buwWIyP8)
Hmm, I am not seeing that @aaronrobertshaw, so not sure what is happening there: I added some additional patterns via the duplicate option in the site editor as it looks like that is what you also did given the naming, and they appeared in the inserter list ok. Maybe see if they are being returned by this selector or not as a starting point for debugging? |
@aaronrobertshaw it could also be worth checking if those patterns are not displaying on trunk, as in theory this PR doesn't touch anything that affects the display in that patterns preview tab. |
They are returned via this selector. They also appear to be still in the returned patterns within
These patterns aren't displaying in trunk either. I was only testing that tab as it was in the PR's test instructions.
Looking closer, I can actually see the patterns in that category panel's markup. Deleting one reveals the next. Also, #51947 introduced a typo in the category description text. |
yeh, it should have been tested as part of this PR for sure - just in case - and turns out it was worth doing to uncover another bug 😄 |
@aaronrobertshaw if you can work out what the issue is and work out a fix, feel free to push it to this PR if not too complex, or if you can work out how to replicate I am happy to help look for a fix. |
There's a Screen.Recording.2023-07-25.at.1.04.08.pm.mp4 |
False alarm, sorry. I was testing with a theme that threw a PHP warning which caused the issue discussed above. |
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.
LGTM 👍
Feel free to get a second opinion on the API changes if you'd like. I think they are ok but don't hold strong opinions there.
What?
Allows adding of user added unsynced patterns from the quick inserter
Why?
Currently it is confusing that a user can add synced patterns and insert them from the quick inserter, but not unsynced patterns
How?
Adds the syncStatus to the inserter reusable blocks so unsynced patterns can be identified and handled differently
There is an additional PR which merges the Synced patterns inserter tab into the My patterns section of the Patterns tab.
Testing Instructions
Screenshots or screencast
quick-inserter.mp4