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

Verse Block: Adds support for custom padding #27341

Merged
merged 6 commits into from
Dec 4, 2020

Conversation

scruffian
Copy link
Contributor

Description

Adds support for custom padding to the Verse block. Also adds a default of 20px to match the padding that the editor adds. We might decide that to remove this padding.

How has this been tested?

Tested in TT1 Blocks in Chrome.

Screenshots

Screenshot 2020-11-27 at 16 44 15

Types of changes

New feature

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.

@scruffian scruffian self-assigned this Nov 27, 2020
@scruffian scruffian added the [Block] Verse Affects the Verse block label Nov 27, 2020
@github-actions
Copy link

github-actions bot commented Nov 27, 2020

Size Change: +382 B (0%)

Total Size: 1.19 MB

Filename Size Change
build/block-editor/index.js 128 kB +22 B (0%)
build/block-editor/style-rtl.css 11.2 kB +56 B (0%)
build/block-editor/style.css 11.2 kB +52 B (0%)
build/block-library/editor-rtl.css 9.07 kB -3 B (0%)
build/block-library/editor.css 9.07 kB -3 B (0%)
build/block-library/index.js 149 kB +28 B (0%)
build/components/style-rtl.css 15.3 kB +24 B (0%)
build/components/style.css 15.3 kB +27 B (0%)
build/edit-site/index.js 24.5 kB +472 B (1%)
build/edit-site/style-rtl.css 3.91 kB -156 B (3%)
build/edit-site/style.css 3.91 kB -155 B (3%)
build/editor/index.js 43.6 kB +18 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.8 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-library/style-rtl.css 8.34 kB 0 B
build/block-library/style.css 8.34 kB 0 B
build/block-library/theme-rtl.css 789 B 0 B
build/block-library/theme.css 790 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 172 kB 0 B
build/compose/index.js 9.95 kB 0 B
build/core-data/index.js 15.2 kB 0 B
build/data-controls/index.js 827 B 0 B
build/data/index.js 8.98 kB 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.95 kB 0 B
build/edit-navigation/index.js 11.1 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.49 kB 0 B
build/edit-post/style.css 6.47 kB 0 B
build/edit-widgets/index.js 26.3 kB 0 B
build/edit-widgets/style-rtl.css 3.13 kB 0 B
build/edit-widgets/style.css 3.13 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.63 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.74 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.27 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keyboard-shortcuts/index.js 2.54 kB 0 B
build/keycodes/index.js 1.93 kB 0 B
build/list-reusable-blocks/index.js 3.1 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.31 kB 0 B
build/notices/index.js 1.82 kB 0 B
build/nux/index.js 3.42 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.43 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 2.92 kB 0 B
build/rich-text/index.js 13.4 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 2.84 kB 0 B
build/viewport/index.js 1.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

So it works. You better check with someone else more familiar with how Global Styles are designed to work but according to my understanding this is correct.

@gziolo
Copy link
Member

gziolo commented Nov 27, 2020

One unit test fails, running npm run fixtures:regenerate should update it and make it pass. There’s now a value set for style. Hmm, it might also update HTML output file which would be a bad behavior as it means that it would invalidate all previously created Verse blocks.

@gziolo
Copy link
Member

gziolo commented Dec 3, 2020

I tried to test with:

<!-- wp:verse {"style":{"typography":{"fontFamily":"var:preset|font-family|system"}}} -->
<pre class="wp-block-verse" style="font-family:var(--wp--preset--font-family--system)">This is my styled verse</pre>
<!-- /wp:verse -->

<!-- wp:verse -->
<pre class="wp-block-verse">This is unstyled verse
</pre>
<!-- /wp:verse -->

One thing that wasn't clear to me is how do I enable custom padding for the theme. If the theme doesn't have the support for custom padding this default value is ignored which feels like a good thing. I figured out that the theme needs to call:

add_theme_support( 'custom-spacing' );

With that added, I was able to test how the test content behaves and it works perfectly fine. Feel free to merge this PR after the rebase.

@scruffian scruffian force-pushed the update/verse-block-padding branch from 7c39a3a to 2ba1a4c Compare December 3, 2020 14:23
@@ -342,6 +342,7 @@ These are the current color properties supported by blocks:
| --- | --- |
| Cover | Yes |
| Group | Yes |
| Verse | Yes |
Copy link
Member

Choose a reason for hiding this comment

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

Thank you! :)

@scruffian
Copy link
Contributor Author

I tried regenerating the fixtures again but it failed. Do I need to add a depreaction?

@oandregal
Copy link
Member

Tried to re-run the tests but something went wrong, not sure what. Could you try to rebase from master and push again so the CI jobs are restarted?

@scruffian scruffian force-pushed the update/verse-block-padding branch from f85d969 to 0b9a7e4 Compare December 3, 2020 20:16
@scruffian
Copy link
Contributor Author

I rebased but the tests failed again.

@scruffian scruffian force-pushed the update/verse-block-padding branch from 0b9a7e4 to ab44b59 Compare December 4, 2020 11:37
@scruffian scruffian merged commit a6e0b02 into master Dec 4, 2020
@scruffian scruffian deleted the update/verse-block-padding branch December 4, 2020 19:07
@github-actions github-actions bot added this to the Gutenberg 9.6 milestone Dec 4, 2020
guarani added a commit that referenced this pull request Dec 7, 2020
guarani added a commit that referenced this pull request Dec 7, 2020
@scruffian scruffian mentioned this pull request Dec 8, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Verse Affects the Verse block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants