Skip to content
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

Block library: Try variations for Group block #20218

Merged
merged 3 commits into from
Mar 31, 2020

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Feb 13, 2020

Description

closes #20200.

This is an experiment that allows using section tag as a wrapper used with the Group block. It uses new block variations API and registers Section block variation to use in the inserter.

Screenshots

Screen Shot 2020-02-13 at 17 49 34
Screen Shot 2020-02-13 at 17 49 46

@gziolo gziolo added [Package] Block library /packages/block-library [Type] Enhancement A suggestion for improvement. labels Feb 13, 2020
@gziolo gziolo requested a review from mtias February 13, 2020 16:54
@gziolo gziolo self-assigned this Feb 13, 2020
@gziolo gziolo requested review from mcsf and kjellr February 13, 2020 16:54
@mtias
Copy link
Member

mtias commented Feb 13, 2020

Seems promising! :)

Copy link
Member

@ZebulanStanphill ZebulanStanphill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I've been waiting for this feature for a long time.

I think it would be a good idea to add a dropdown to change the tag in the Inspector, so you can change the tag on existing blocks. Actually, maybe this is something that could even be placed in the toolbar.

@@ -13,6 +13,10 @@
},
"customTextColor": {
"type": "string"
},
"tagName": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should probably be some way of limiting this attribute to certain whitelisted values (div, section, header, footer, and aside are what I'm thinking).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking it could be less of a hard limit and more of a recommendation while in the editor, similarly to how the editor advises against incorrect heading levels or against low-contrast colour combinations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I imagine it could also have support as a wrapper for custom-elements.

@mcsf
Copy link
Contributor

mcsf commented Feb 18, 2020

I think it would be a good idea to add a dropdown to change the tag in the Inspector, so you can change the tag on existing blocks. Actually, maybe this is something that could even be placed in the toolbar.

This is something to think about in the broad context of block variations (and patterns?). I do believe there are some interesting opportunities in terms of how a block is aware of its own "degree of variation" from the base block type or chosen variation, and offering to convert in-place between variations.

@mcsf
Copy link
Contributor

mcsf commented Feb 18, 2020

This is nifty! What do you see is left to do, @gziolo?

@gziolo
Copy link
Member Author

gziolo commented Feb 18, 2020

I think it would be a good idea to add a dropdown to change the tag in the Inspector, so you can change the tag on existing blocks. Actually, maybe this is something that could even be placed in the toolbar.

This is something to think about in the broad context of block variations (and patterns?). I do believe there are some interesting opportunities in terms of how a block is aware of its own "degree of variation" from the base block type or chosen variation, and offering to convert in-place between variations.

It's something that it's on the radar. It needs some experimentation because in some cases it might cause content loss. We discussed some strategies with @mcsf how to mitigate it and one possible solution would be to show a snackbar notice with an option to revert changes. In the case of this proposal, it would be safe to allow changing block variations.

This is nifty! What do you see is left to do, @gziolo?

The proposed variation could have a unique icon and description. In addition, we might want to cover more variations listed in the parent issue, e.g. header, main, aside. They should be very useful in the context of full site editing. It also makes me think that maybe variations should also support the parent field.

@youknowriad
Copy link
Contributor

In an attempt to move this forward, I've rebased it and make the required adjustements.

IMO, this is ready for a v1, the question for me remains about the variations: What variations do we want to expose initially if any. Right now there's just a "section" variation, but let me know if you think we should add more now.

@github-actions
Copy link

github-actions bot commented Mar 19, 2020

Size Change: +77 B (0%)

Total Size: 866 kB

Filename Size Change
build/block-editor/index.js 102 kB +22 B (0%)
build/block-library/index.js 111 kB +55 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 998 B 0 B
build/annotations/index.js 3.44 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 11 kB 0 B
build/block-library/editor-rtl.css 7.21 kB 0 B
build/block-library/editor.css 7.21 kB 0 B
build/block-library/style-rtl.css 7.5 kB 0 B
build/block-library/style.css 7.51 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 191 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.7 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.26 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-navigation/index.js 2.4 kB 0 B
build/edit-navigation/style-rtl.css 95 B 0 B
build/edit-navigation/style.css 95 B 0 B
build/edit-post/index.js 92.3 kB 0 B
build/edit-post/style-rtl.css 8.35 kB 0 B
build/edit-post/style.css 8.34 kB 0 B
build/edit-site/index.js 8.65 kB 0 B
build/edit-site/style-rtl.css 3.46 kB 0 B
build/edit-site/style.css 3.46 kB 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 2.57 kB 0 B
build/edit-widgets/style.css 2.57 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 426 B 0 B
build/editor/index.js 42.8 kB 0 B
build/editor/style-rtl.css 3.38 kB 0 B
build/editor/style.css 3.38 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 712 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 780 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.6 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

'section',
'aside',
'footer',
'main',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you would do with form since there are generally no granular blocks to make sense of it. Blocks that perform form-like functions would already define the element for themselves.

@gziolo
Copy link
Member Author

gziolo commented Mar 19, 2020

There are some unit tests failing, we need to investigate further whether it's fine to regenerate snapshots or we need to add deprecation entry for the block.

@ZebulanStanphill
Copy link
Member

@youknowriad

What variations do we want to expose initially if any. Right now there's just a "section" variation, but let me know if you think we should add more now.

I think aside, header, and footer would be good to expose intitially. I've used aside in a few of my blog posts (and had to use the Custom HTML block for the entire thing), and it would be nice to just use the Group block so I can keep using blocks for the nested paragraphs. header and footer will both be useful in the context of full site editing, and for those looking for extra semantic markup, those elements can be used for the headers and footers of document sub-sections made using the section element.

@youknowriad
Copy link
Contributor

I'm still hesitant on the variations here, they have the potential to confuse regular users. So I'm thinking we should just remove the variations for now and merge the PR to move forward and allow FSE themes to use the new attribute.

Copy link
Member

@ZebulanStanphill ZebulanStanphill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test failures actually look related (maybe the branch needs to be rebased?), but other than that, I think this is good to go. Just having the attribute that I can manually edit will allow me to start using Group blocks in some places on my site where I'm currently using Custom HTML blocks. 🙂

@gziolo
Copy link
Member Author

gziolo commented Mar 30, 2020

I found the reason, I will push the fix tomorrow and land this PR. The migration function needs to set tagName to the default value. I'm not sure why it isn't automatically exposed. Well, maybe it makes sense since this attribute wasn't defined earlier.

gziolo and others added 3 commits March 31, 2020 11:16
This is an experiment which allows to use `section` tag as a wrapper used with the Group block. It uses new block variations API.
@gziolo
Copy link
Member Author

gziolo commented Mar 31, 2020

All issues resolved, Travis seems to be happy now. We can land this PR once the last job on Travis finishes.

@youknowriad youknowriad merged commit e16260f into master Mar 31, 2020
@youknowriad youknowriad deleted the update/group-variations branch March 31, 2020 10:19
@youknowriad
Copy link
Contributor

Thanks @gziolo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Group Affects the Group Block [Package] Block library /packages/block-library [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow blocks to render semantic tags for full-site editing
6 participants