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

Adds in viewports or adjusts them for block patterns #27204

Merged
merged 4 commits into from
Nov 25, 2020

Conversation

karmatosed
Copy link
Member

Ideally when browsing block patterns you should see as close to what the pattern is going to be, if it's visually represented. Currently the following is what you see:

Before

Problem

The main problem with this is that everything seems 'full width', even when it needs to be set. For example, headers, columns. This can also look a lot better with some spacing, to allow the content to breathe and be understood.

Solution

Props to @kjellr as I was exploring, for mentioning that this could be solved by setting 'viewportsWidth' on block patterns. In exploring I discovered some block patterns didn't have these set, others have very low settings. I have set in this PR some to 1200 along with giving some missing values, to bring a little uniformity.

After applying this PR here is what you should see:

After

Feedback

I would appreciate a code review and design feedback. This is a relatively small pickup PR, so be great to see about getting this in from there.

@karmatosed karmatosed added [Type] Enhancement A suggestion for improvement. [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Nov 23, 2020
@karmatosed karmatosed requested a review from kjellr November 23, 2020 14:46
@github-actions
Copy link

github-actions bot commented Nov 23, 2020

Size Change: -389 B (0%)

Total Size: 1.2 MB

Filename Size Change
build/api-fetch/index.js 3.42 kB +1 B
build/autop/index.js 2.84 kB +2 B (0%)
build/blob/index.js 664 B -1 B
build/block-directory/index.js 8.72 kB +1 B
build/block-editor/index.js 134 kB +316 B (0%)
build/block-editor/style-rtl.css 11.3 kB -28 B (0%)
build/block-editor/style.css 11.3 kB -28 B (0%)
build/block-library/editor-rtl.css 8.95 kB -12 B (0%)
build/block-library/editor.css 8.95 kB -12 B (0%)
build/block-library/index.js 148 kB +330 B (0%)
build/block-serialization-default-parser/index.js 1.87 kB -1 B
build/blocks/index.js 48.1 kB +19 B (0%)
build/components/index.js 172 kB +69 B (0%)
build/compose/index.js 9.95 kB +20 B (0%)
build/core-data/index.js 14.8 kB -1 B
build/data/index.js 8.8 kB -915 B (10%) 👏
build/date/index.js 11.2 kB -2 B (0%)
build/deprecated/index.js 768 B -1 B
build/dom/index.js 4.95 kB +34 B (0%)
build/edit-navigation/index.js 11.1 kB -39 B (0%)
build/edit-post/index.js 306 kB +115 B (0%)
build/edit-post/style-rtl.css 6.42 kB -36 B (0%)
build/edit-post/style.css 6.4 kB -35 B (0%)
build/edit-site/index.js 23.6 kB +123 B (0%)
build/edit-site/style-rtl.css 3.92 kB +62 B (1%)
build/edit-site/style.css 3.92 kB +61 B (1%)
build/edit-widgets/index.js 26.3 kB -122 B (0%)
build/editor/index.js 43.3 kB -1 B
build/element/index.js 4.62 kB +2 B (0%)
build/format-library/index.js 6.86 kB +4 B (0%)
build/i18n/index.js 3.57 kB -1 B
build/keyboard-shortcuts/index.js 2.54 kB -308 B (12%) 👏
build/keycodes/index.js 1.94 kB -1 B
build/list-reusable-blocks/index.js 3.1 kB -2 B (0%)
build/media-utils/index.js 5.32 kB +2 B (0%)
build/notices/index.js 1.81 kB -1 B
build/plugins/index.js 2.56 kB -3 B (0%)
build/primitives/index.js 1.43 kB +1 B
build/redux-routine/index.js 2.84 kB -2 B (0%)
build/reusable-blocks/index.js 2.92 kB +2 B (0%)
build/rich-text/index.js 13.3 kB -4 B (0%)
build/server-side-render/index.js 2.77 kB -1 B
build/shortcode/index.js 1.69 kB +1 B
build/viewport/index.js 1.86 kB +2 B (0%)
build/warning/index.js 1.14 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.8 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.23 kB 0 B
build/block-library/style.css 8.23 kB 0 B
build/block-library/theme-rtl.css 792 B 0 B
build/block-library/theme.css 793 B 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/data-controls/index.js 827 B 0 B
build/dom-ready/index.js 571 B 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 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.85 kB 0 B
build/escape-html/index.js 735 B 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/is-shallow-equal/index.js 698 B 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 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/priority-queue/index.js 790 B 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.05 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@kjellr
Copy link
Contributor

kjellr commented Nov 23, 2020

This seems like a good improvement!

Would it make sense to set a wider default viewport though? That way future blocks would benefit from this by default.

@karmatosed karmatosed requested a review from ellatrix as a code owner November 24, 2020 14:54
@karmatosed
Copy link
Member Author

@kjellr your comment took me on a journey into the code. I wasn't aware it wasn't set individually, so that's great information on block patterns. I have increased the default to 1200, which also means can reduce back a few lines, as they all use default value.

@@ -7,7 +7,8 @@

return array(
'title' => __( 'Two columns of text with images', 'gutenberg' ),
'categories' => array( 'columns' ),
'categories' => array( 'columns' ),
'viewportWidth' => 1200,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this one be taken out too?

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

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

Left a single comment, but other than that this looks good to me! Thanks for looking into this.

@karmatosed
Copy link
Member Author

Thanks for code review, caught that final one (so many lines we got to save here!). Once tests have passed I'll merge this little fix. Thanks @kjellr for collaborating on this.

@karmatosed
Copy link
Member Author

@ItsJonQ thank you so much it seems that the tests are all starting to pass now!

@karmatosed karmatosed merged commit 2d2bfcf into master Nov 25, 2020
@karmatosed karmatosed deleted the try/viewport-iterations-patterns branch November 25, 2020 17:08
@github-actions github-actions bot added this to the Gutenberg 9.5 milestone Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants