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

Template part: attribute feature parity with group block #25029

Merged
merged 3 commits into from
Sep 3, 2020

Conversation

noahtallen
Copy link
Member

@noahtallen noahtallen commented Sep 2, 2020

Description

Closes #20997 and resolves #22034. Adds all attributes which currently exist on the group block to the template part block. (Including light block wrapper and tagName support.)

Note: These are just block attributes at the block level. Attributes are not persisted to a template part globally. This means that a change to these block attributes only applies to where the template part block is used, likely on a template level, rather than a template part level.

How has this been tested?

Locally in edit site.

Screenshots

Screenshot shows a custom HTML tagName, custom background color, and custom text color in editor and then on the front end. The margins are a separate issue :D

Front end:

Screen Shot 2020-09-02 at 3 59 47 PM

Editor:

Screen Shot 2020-09-02 at 3 59 01 PM

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

github-actions bot commented Sep 2, 2020

Size Change: +111 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/block-editor/index.js 128 kB +51 B (0%)
build/block-library/index.js 138 kB +35 B (0%)
build/block-library/theme-rtl.css 754 B +13 B (1%)
build/block-library/theme.css 754 B +12 B (1%)
ℹ️ 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.41 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.99 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.64 kB 0 B
build/block-library/editor.css 8.64 kB 0 B
build/block-library/style-rtl.css 7.6 kB 0 B
build/block-library/style.css 7.6 kB 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/blocks/index.js 47.7 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.55 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 4.48 kB 0 B
build/edit-navigation/index.js 11.7 kB 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/index.js 305 kB 0 B
build/edit-post/style-rtl.css 6.26 kB 0 B
build/edit-post/style.css 6.25 kB 0 B
build/edit-site/index.js 17.1 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/index.js 12 kB 0 B
build/edit-widgets/style-rtl.css 2.46 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.6 kB 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 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.71 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.57 kB 0 B
build/is-shallow-equal/index.js 710 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.12 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.32 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.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 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

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 and it works well minus a couple things:

  1. The placeholder component is now taking up full width in the editor. This is probably a result of switching to the light wrapper without adding a wrapper around the Placeholders return as well.

Screen Shot 2020-09-02 at 8 22 18 PM

  1. I can't find a way to change the tagName? I am assuming this is something we will need to add manually.

@noahtallen
Copy link
Member Author

I can't find a way to change the tagName

I don't think this is supported by any blocks right now. I think you just define it in the block HTML

@Addison-Stavlo
Copy link
Contributor

I don't think this is supported by any blocks right now. I think you just define it in the block HTML

Ah interesting, I think we should definitely add a way to set this in a followup... it seems like it should be necessary. 🤔

@noahtallen
Copy link
Member Author

Yeah, true. I wonder if there are existing issues for that for the group block and others

@noahtallen
Copy link
Member Author

The placeholder component is now taking up full width in the editor. This is probably a result of switching to the light wrapper without adding a wrapper around the Placeholders return as well.

Added the wrapper there and it seems to be fixed ✅

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.

Added the wrapper there and it seems to be fixed ✅

Works for me. 😁

@youknowriad
Copy link
Contributor

According to the screenshot, it seems we're missing the has-background style (padding) for this block?

@noahtallen
Copy link
Member Author

I think I do see it on the front end (from that screenshot):

Screen Shot 2020-09-03 at 10 26 28 AM

Are there other classnames that might add margin there? Maybe the group block has some default margin/padding styles we need to add to the template part block?

@noahtallen noahtallen force-pushed the try/template-part-more-attributes branch from b1a52fd to 7535c10 Compare September 3, 2020 18:16
@noahtallen
Copy link
Member Author

@youknowriad I did some investigation, and those styles are defined here in the group block, so they won't apply to template parts out of the box:

https://github.com/WordPress/gutenberg/blob/0b93ad47797a1ba2cea3c83a7f973d33abcd351f/packages/block-library/src/group/theme.scss#L1-L10

I'm not sure how to solve this, since it seems there are several styles scattered around to support padding/margin for the group block. I'm not sure exactly what we should change. For example, with the theme I'm testing, it seems the group block padding is ultimately set by the theme, and the theme does not set anything for template part blocks yet.

@noahtallen
Copy link
Member Author

I'm not sure how to solve this

I've copied over the theme.scss file from the group block. So this should provide good baseline styles. but it still seems like padding/margin is primarily something themes need to code?

At any rate, I'll merge what we have since it seems to work well. As always, I'm happy to address feedback in followups

@noahtallen noahtallen merged commit 2da8da9 into master Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[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.

Template part controls should be identical to a group block Template Parts should allow setting HTML tag
3 participants