Add content schema to pattern editing view #59977
Merged
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
talldan
added
[Type] Enhancement
|
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: +191 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
Mamaduka
reviewed
Mar 19, 2024
Comment on lines
20
to
24
| const allClientIds = useSelect( | ||
| ( select ) => | ||
| unlock( select( blockEditorStore ) ).getClientIdsWithDescendants(), | ||
| [] | ||
| ); |
There was a problem hiding this comment.
I think getClientIdsWithDescendants is a public selector, so unlocking it is unnecessary.
You can also use a filter inside the mapSelect if you just return clientId strings. While it's against general policy to avoid filter/map/etc iteration inside selectors. I think it's a useful "trick". Here's a similar example for the locked block.
Note to self: it might be helpful to include comments for similar "tricks" explaining why this usage is okay.
There was a problem hiding this comment.
I think getClientIdsWithDescendants is a public selector, so unlocking it is unnecessary.
Thanks, not sure why I did that 😄
You can also use a filter inside the mapSelect if you just return clientId strings. While it's against general policy to avoid filter/map/etc iteration inside selectors. I think it's a useful "trick". Here's a similar example for the locked block.
Interesting. Could you explain more about how that works without causing a performance hit?
In my testing it seems like it would cause a performance problem because useSelect runs so regularly even when it's returning the same value.
There was a problem hiding this comment.
That's true; the mapSelect will run whenever the store updates. To be honest, I've not measured performance hits for similar patterns. Though it should be fine for simple filter operations.
I trust to your judgment here :)
22 tasks
kevin940726
approved these changes
Mar 20, 2024
|
Could we rename the sidebar from "Content" to "Overrides" or something specific to patterns? Right now, having it fully mimic the content only editing experience is throwing me a bit and having the UI be a bit more specific to this experience might help. |
|
I'd be fine with using the 'Overrides" label instead of Content for now. |
talldan
force-pushed
the
add/schema-to-pattern-editing-view
branch
from
5c5d54a
to
c0c23fd
Compare
|
I've been testing this, I like how it feels. One unrelated thing that could be worth adding when we're in the pattern editor, is an "info" message on the modal to rename a block for blocks. |
youknowriad
approved these changes
Apr 2, 2024
|
@youknowriad Yep, I think that's being covered by a separate issue (#59813), and @kevin940726 has worked on some options. |
|
Sorry folks, I forgot to add co-authors to the commit message. |
cbravobernal
pushed a commit
to garridinsi/gutenberg
that referenced
this pull request
Apr 9, 2024
* Add new quick navigation component for pattern schema * Implement panel and fix minor issues * Rename to pattern content, and avoid showing panel when there are no blocks * Don't try to unlock what's already unlocked, just turn the handle. * Rename content panel title from 'Content' to 'Overrides' * Rename component to `OverridesPanel`
10 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?
Closes #59815
Adds a new 'content' panel when editing patterns. When a user names a block to declare it as overridable, that block will now display in the panel.
Questions:
How?
ContentPanelcomponent in thepatternspackage, with the various UI piecesPatternContentPanelcomponent in theeditorpackage, which determines when the panel should be shown (whether awp_blockis being edited)isOverridableBlockfunction to the patterns package, which acts as the source of truth for whether a block has overrides (which is important as this will likely be changing with Use metadata.name only as the hint for pattern overrides)Testing Instructions
Screenshots or screencast
Kapture.2024-03-19.at.12.46.29.mp4