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

Change 'Two images' pattern to wide width #23856

Merged
merged 2 commits into from
Jul 20, 2020

Conversation

enriquesanchez
Copy link
Contributor

Updates the 'Two images' pattern to be wide width.

Before:
Screen Shot 2020-07-09 at 19 54 24

After:
Screen Shot 2020-07-09 at 19 54 37

One issue after this update is that the images are not high-res enough for this size, especially the left one. Considering that we are constrained by the images we can source for the Core patterns library in Core, it might be OK to go with this until we come up with a solution re: images.

@enriquesanchez enriquesanchez added the [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced label Jul 10, 2020
@enriquesanchez enriquesanchez self-assigned this Jul 10, 2020
@github-actions
Copy link

github-actions bot commented Jul 10, 2020

Size Change: +419 B (0%)

Total Size: 1.14 MB

Filename Size Change
build/api-fetch/index.js 3.39 kB +1 B
build/block-editor/index.js 115 kB -17 B (0%)
build/block-editor/style-rtl.css 10.8 kB +7 B (0%)
build/block-editor/style.css 10.8 kB +4 B (0%)
build/block-library/index.js 132 kB +36 B (0%)
build/blocks/index.js 48.2 kB -1 B
build/components/index.js 199 kB -25 B (0%)
build/compose/index.js 9.67 kB +1 B
build/core-data/index.js 11.5 kB +12 B (0%)
build/edit-navigation/index.js 10.8 kB +15 B (0%)
build/edit-post/index.js 304 kB +163 B (0%)
build/edit-post/style-rtl.css 5.6 kB +13 B (0%)
build/edit-post/style.css 5.6 kB +13 B (0%)
build/edit-site/index.js 16.8 kB +173 B (1%)
build/edit-site/style-rtl.css 3.04 kB +11 B (0%)
build/edit-site/style.css 3.04 kB +12 B (0%)
build/edit-widgets/index.js 9.35 kB -4 B (0%)
build/editor/index.js 45.1 kB +2 B (0%)
build/format-library/index.js 7.72 kB +3 B (0%)
build/keyboard-shortcuts/index.js 2.51 kB +1 B
build/list-reusable-blocks/index.js 3.12 kB +1 B
build/media-utils/index.js 5.32 kB +1 B
build/rich-text/index.js 13.9 kB -1 B
build/token-list/index.js 1.27 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 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.67 kB 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-library/editor-rtl.css 7.6 kB 0 B
build/block-library/editor.css 7.59 kB 0 B
build/block-library/style-rtl.css 7.77 kB 0 B
build/block-library/style.css 7.77 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 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.9 kB 0 B
build/components/style.css 15.8 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.46 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 569 B 0 B
build/dom/index.js 3.23 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-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/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 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.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keycodes/index.js 1.94 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/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.4 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/server-side-render/index.js 2.71 kB 0 B
build/shortcode/index.js 1.7 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.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@kjellr
Copy link
Contributor

kjellr commented Jul 10, 2020

One issue after this update is that the images are not high-res enough for this size, especially the left one. Considering that we are constrained by the images we can source for the Core patterns library in Core, it might be OK to go with this until we come up with a solution re: images.

Hmm — this doesn't seem ok to me. We shouldn't include something pixelly... If we don't have any larger versions to draw from, maybe we swap with a different image from the Core patterns library for now? (if there are other good options there?)

@enriquesanchez
Copy link
Contributor Author

Hmm — this doesn't seem ok to me. We shouldn't include something pixelly

@kjellr Thanks, and I think you're right.

The other images we already use in the block and patterns library are also not high-res enough for this big of a size. While less noticeable, the pixelation is still there:

Screen Shot 2020-07-10 at 13 42 28

I'd be more than glad to donate a couple of images I've taken, but I worry this will lead to a larger discussion about sourcing images:

Screen Shot 2020-07-10 at 13 54 44

@mapk
Copy link
Contributor

mapk commented Jul 10, 2020

Those photos of the mountains are amazing, Enrique! But I too fear this will lead into another long discussion about whether those are the right images. @kjellr what do you think. If we keep the lower rez images, we should keep the block set for the default width. If we want to go wider, we'll need new images.

@youknowriad youknowriad added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 13, 2020
@kjellr
Copy link
Contributor

kjellr commented Jul 13, 2020

I think the wider images are much nicer, but we definitely can't have them be pixelly. I'm sure we have higher-resolution versions of those same images — can we not upload those?

@enriquesanchez
Copy link
Contributor Author

I've requested to update the images for higher-res versions here: https://core.trac.wordpress.org/ticket/50650

Change the image URLs to reflect the proposed new location from the Core Trac ticket.
@desrosj
Copy link
Contributor

desrosj commented Jul 14, 2020

Since images cannot be replaced on the CDN reliably (caching is too persistent), the plan proposed on the Core Trac ticket is to use the same image names and just introduce them into a new folder, core/5.5. I updated this PR accordingly.

@enriquesanchez
Copy link
Contributor Author

Thanks for the update @desrosj! 🙏

@kjell @mapk New images from the wp.org CDN are higher res and all should look much better now:

Screen Shot 2020-07-16 at 19 05 10

Copy link
Contributor

@mapk mapk left a comment

Choose a reason for hiding this comment

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

The images work great! No pixelation. Should we update the pattern to be full width? Right now it's at wide. I'm approving just in case you decide to keep it at just wide. :)

@enriquesanchez enriquesanchez merged commit 557a41b into master Jul 20, 2020
@enriquesanchez enriquesanchez deleted the update-two-images-pattern branch July 20, 2020 18:13
@github-actions github-actions bot added this to the Gutenberg 8.7 milestone Jul 20, 2020
@youknowriad youknowriad removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 27, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants