Move the "HTML element" control to a block editor component #41133
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
|
Size Change: -531 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
aristath
reviewed
May 18, 2022
Mamaduka
reviewed
May 18, 2022
carolinan
requested review from
ellatrix,
michalczaplinski and
ajitbohra
as code owners
Mamaduka
reviewed
May 19, 2022
Comment on lines
55
to
56
| tagNameOptions = DEFAULT_OPTIONS, | ||
| selectedValue, |
There was a problem hiding this comment.
@carolinan, what do you think about using options and value props like SelectControl?
Mamaduka
reviewed
May 19, 2022
Mamaduka
reviewed
May 19, 2022
Comment on lines
43
to
54
| ```js | ||
| [ | ||
| { label: '<div>', value: 'div' }, | ||
| { label: '<header>', value: 'header' }, | ||
| { label: '<main>', value: 'main' }, | ||
| { label: '<section>', value: 'section' }, | ||
| { label: '<article>', value: 'article' }, | ||
| { label: '<aside>', value: 'aside' }, | ||
| { label: '<footer>', value: 'footer' }, | ||
| { label: '<nav>', value: 'nav' }, | ||
| ]; | ||
| ``` |
There was a problem hiding this comment.
We should fix the alignment for this example.
I don't think that introducing a new tiny wrapper component brings that much value as presented in this PR. I initially was happy to see this exploration, but I noticed that the actual implementation doesn't fulfill the promise of removing code duplication. There are 4 places where
|
Co-authored-by: George Mamadashvili <[email protected]>
|
Yes, it is worth having a discussion about what tags should be available, and for which blocks. We could also allow the blocks to use all the listed tags without limitation. I am not convinced that it is the best solution. Group - Container, there are valid cases for the defaults. Template parts - Container, there are valid cases for the defaults. Has a custom option that I do not know how to combine with the defaults: Comments - I can't think of valid cases where this would be a header or footer. Query - I can't think of valid cases where this would be a header or footer. Blocks that do not have the option yet: Columns and cover blocks are also containers. |
Maybe instead of introducing a new component in the public API of |
I think that's a great idea!
What do you think @gziolo? Would a structure like the one above make sense? |
Yes! I like the general idea. I wouldn't commit to the exact API without seeing the implementation together with refactored core blocks, but I'm 100% on board with giving it a try. It's exactly what I was thinking about as it resembles how we handle custom class names or anchors:
For example, the code for the custom class name lives here: |
|
Great! thank you for the feedback Greg, and the reference to the class-name code 👍 |
|
I have started, but I am struggling with the save function. How do I update the actual html tag? (and not an attribute) |
|
This is the branch I am testing it on: |
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?
Adds a new component for selecting a HTML tag for the TagName block attribute.
Partial for #33343
Why?
The control for selecting a HTML tag for a block is re-used in a couple of blocks and by creating a block editor component for it we reduce the duplicated code and it will be easier to update in the future.
How?
with a default list of selectable HTML tags and helptext.
Testing Instructions
Activate Twenty Twenty-two and open the site editor (Reset any customizations).
Viewing the home template, add a comments block.
Confirm that:
-The header template part correctly uses the tagName it has been assigned:
header-The first group block in the root level of the template correctly uses the
mainelement.-You can change the header template part, the group block, comments and the query block to any element listed in the "HTML element" control in the Advanced section of the block settings sidebar.
-Help text is displaying correctly below the control.
Save and reload the site editor, confirm that there are no block validation errors or other issues.
View the front, confirm that the blocks use the correct tag.
Screenshots or screencast