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

Implement the custom classname support hook in the server #24483

Merged
merged 4 commits into from
Aug 17, 2020

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Aug 11, 2020

In different dynamic blocks, we support custom class names in the server in an Adhoc way and now that we have the block-supports handling on the server as well, this PR implements it automatically for all these blocks.

There are still other hooks that could benefit from a similar approach: anchor, generated classname.

Testing instructions

Check that custom classnames work for these blocks (editor and frontend)

  • archives, calendar, tag-cloud, categories, latest comments, latest posts, navigation, rss, and search.

@youknowriad youknowriad added [Feature] Block API API that allows to express the block paradigm. [Type] New API New API to be used by plugin developers or package users. labels Aug 11, 2020
@youknowriad youknowriad self-assigned this Aug 11, 2020
@youknowriad youknowriad force-pushed the add/custom-classname-server branch from 77141d7 to 7e93d38 Compare August 11, 2020 13:12
@github-actions
Copy link

github-actions bot commented Aug 11, 2020

Size Change: +3.33 kB (0%)

Total Size: 1.16 MB

Filename Size Change
build/block-editor/index.js 125 kB +123 B (0%)
build/block-editor/style-rtl.css 10.6 kB -13 B (0%)
build/block-editor/style.css 10.6 kB -13 B (0%)
build/block-library/editor-rtl.css 8.36 kB +771 B (9%) 🔍
build/block-library/editor.css 8.36 kB +771 B (9%) 🔍
build/block-library/index.js 132 kB -174 B (0%)
build/block-library/style-rtl.css 7.49 kB -278 B (3%)
build/block-library/style.css 7.49 kB -279 B (3%)
build/blocks/index.js 48.4 kB +1 B
build/components/index.js 200 kB +25 B (0%)
build/core-data/index.js 11.8 kB -4 B (0%)
build/edit-post/style-rtl.css 5.63 kB +19 B (0%)
build/edit-post/style.css 5.63 kB +19 B (0%)
build/edit-widgets/index.js 11.7 kB +2.36 kB (20%) 🚨
build/url/index.js 4.06 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.97 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.68 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.45 kB 0 B
build/date/index.js 5.38 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.23 kB 0 B
build/edit-navigation/index.js 10.9 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.33 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.71 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@youknowriad youknowriad added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Aug 11, 2020
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

Tested this out on 'Site Title' block and can verify the custom class name is now applied correctly on the front-end! 🎉

This looks good to me, as it follows the standard convention for the rest of the supported styles (minus the slight change to default true since it is an opt-out setting?)

I assume we should probably expand the test file to include this as well? Generally for each style I have added 1 test to ensure the style is added when expected, 1 test to ensure the style isnt added when unsupported, and generally update the all-applied test to include it as well. With a lot of styles being supported this convention is growing the file quite a bit, so maybe you have other ideas? Although, it is simple enough to create new ones with copy/paste and minimal alteration as the format is very similar for them all. 🤷‍♀️

@youknowriad
Copy link
Contributor Author

Thanks for testing @Addison-Stavlo I've added a unit test.

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

Nice, it looks good to me!

I also went through and tested the noted blocks:

archives, calendar, tag-cloud, categories, latest comments, latest posts, navigation, rss, and search.

And all are having custom classnames applied as expected. 🎉

@youknowriad youknowriad merged commit ad48d82 into master Aug 17, 2020
@youknowriad youknowriad deleted the add/custom-classname-server branch August 17, 2020 07:36
@github-actions github-actions bot added this to the Gutenberg 8.8 milestone Aug 17, 2020
@cguntur cguntur mentioned this pull request Oct 14, 2020
17 tasks
@tellthemachines tellthemachines removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Type] New API New API to be used by plugin developers or package users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants