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

Gallery: Convert Gallery block to use Image blocks instead of having its own nested image format #25940

Merged
merged 158 commits into from Aug 19, 2021

Conversation

@glendaviesnz
Copy link
Contributor

@glendaviesnz glendaviesnz commented Oct 8, 2020

Fixes #10057

Description

Refactors the Gallery block to be a wrapper around Image blocks using InnerBlocks.

gallery

Taking this approach eliminates the current differences between stand alone image blocks, and images within the gallery, and can hopefully avoid a lot of duplication of image functionality between the various gallery blocks.

See previous PoC PR for some background discussion

Testing

  • This PR can be tested with adding new Galleries and making sure existing galleries still display in the old formate
  • Check out the PR and add or view a gallery. It should still be in the old format
  • Turn on the gutenberg experimental feature 'Gallery refactor' and any new galleries added should now be made up of nested Image blocks.
  • Related PR #28481 has the block fixture and unit test updates - separated out to help make review easier

Types of changes

Significant changes to structure of Gallery block to remove image metadata and replace with Image blocks via InnerBlocks

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 github-actions bot commented Oct 8, 2020

Size Change: +11.2 kB (+1%)

Total Size: 1.04 MB

Filename Size Change
build/block-editor/index.min.js 119 kB +124 B (0%)
build/block-editor/style-rtl.css 13.8 kB -28 B (0%)
build/block-editor/style.css 13.8 kB -24 B (0%)
build/block-library/blocks/gallery/editor-rtl.css 879 B +172 B (+24%) 🚨
build/block-library/blocks/gallery/editor.css 876 B +170 B (+24%) 🚨
build/block-library/blocks/gallery/style-rtl.css 1.7 kB +644 B (+61%) 🆘
build/block-library/blocks/gallery/style.css 1.7 kB +641 B (+61%) 🆘
build/block-library/blocks/post-featured-image/editor-rtl.css 398 B -14 B (-3%)
build/block-library/blocks/post-featured-image/editor.css 398 B -14 B (-3%)
build/block-library/common-rtl.css 1.29 kB +458 B (+55%) 🆘
build/block-library/common.css 1.29 kB +458 B (+55%) 🆘
build/block-library/editor-rtl.css 9.98 kB +599 B (+6%) 🔍
build/block-library/editor.css 9.97 kB +601 B (+6%) 🔍
build/block-library/index.min.js 150 kB +3.46 kB (+2%)
build/block-library/style-rtl.css 11 kB +1.18 kB (+12%) ⚠️
build/block-library/style.css 11 kB +1.18 kB (+12%) ⚠️
build/blocks/index.min.js 47 kB +8 B (0%)
build/components/index.min.js 209 kB +86 B (0%)
build/data/index.min.js 7.07 kB +46 B (+1%)
build/edit-navigation/index.min.js 13.6 kB +270 B (+2%)
build/edit-navigation/style-rtl.css 3.19 kB +98 B (+3%)
build/edit-navigation/style.css 3.19 kB +94 B (+3%)
build/edit-post/index.min.js 28.6 kB +202 B (+1%)
build/edit-post/style-rtl.css 7.23 kB +50 B (+1%)
build/edit-post/style.css 7.22 kB +50 B (+1%)
build/edit-site/index.min.js 26.2 kB +235 B (+1%)
build/edit-site/style-rtl.css 5.07 kB +63 B (+1%)
build/edit-site/style.css 5.07 kB +63 B (+1%)
build/edit-widgets/index.min.js 16 kB +192 B (+1%)
build/edit-widgets/style-rtl.css 4.06 kB +47 B (+1%)
build/edit-widgets/style.css 4.06 kB +46 B (+1%)
build/editor/index.min.js 37.6 kB +18 B (0%)
build/element/index.min.js 3.17 kB +7 B (0%)
build/rich-text/index.min.js 10.6 kB +18 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 931 B
build/admin-manifest/index.min.js 1.09 kB
build/annotations/index.min.js 2.7 kB
build/api-fetch/index.min.js 2.19 kB
build/autop/index.min.js 2.08 kB
build/blob/index.min.js 459 B
build/block-directory/index.min.js 6.21 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 58 B
build/block-library/blocks/audio/editor.css 58 B
build/block-library/blocks/audio/style-rtl.css 111 B
build/block-library/blocks/audio/style.css 111 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 474 B
build/block-library/blocks/button/editor.css 474 B
build/block-library/blocks/button/style-rtl.css 605 B
build/block-library/blocks/button/style.css 604 B
build/block-library/blocks/buttons/editor-rtl.css 315 B
build/block-library/blocks/buttons/editor.css 315 B
build/block-library/blocks/buttons/style-rtl.css 370 B
build/block-library/blocks/buttons/style.css 370 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 90 B
build/block-library/blocks/code/style.css 90 B
build/block-library/blocks/code/theme-rtl.css 131 B
build/block-library/blocks/code/theme.css 131 B
build/block-library/blocks/columns/editor-rtl.css 194 B
build/block-library/blocks/columns/editor.css 193 B
build/block-library/blocks/columns/style-rtl.css 474 B
build/block-library/blocks/columns/style.css 475 B
build/block-library/blocks/cover/editor-rtl.css 666 B
build/block-library/blocks/cover/editor.css 670 B
build/block-library/blocks/cover/style-rtl.css 1.23 kB
build/block-library/blocks/cover/style.css 1.23 kB
build/block-library/blocks/embed/editor-rtl.css 488 B
build/block-library/blocks/embed/editor.css 488 B
build/block-library/blocks/embed/style-rtl.css 400 B
build/block-library/blocks/embed/style.css 400 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 322 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 159 B
build/block-library/blocks/group/editor.css 159 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 93 B
build/block-library/blocks/group/theme.css 93 B
build/block-library/blocks/heading/editor-rtl.css 152 B
build/block-library/blocks/heading/editor.css 152 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/home-link/style-rtl.css 247 B
build/block-library/blocks/home-link/style.css 247 B
build/block-library/blocks/html/editor-rtl.css 283 B
build/block-library/blocks/html/editor.css 284 B
build/block-library/blocks/image/editor-rtl.css 728 B
build/block-library/blocks/image/editor.css 728 B
build/block-library/blocks/image/style-rtl.css 482 B
build/block-library/blocks/image/style.css 487 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B
build/block-library/blocks/latest-posts/editor.css 137 B
build/block-library/blocks/latest-posts/style-rtl.css 528 B
build/block-library/blocks/latest-posts/style.css 527 B
build/block-library/blocks/list/style-rtl.css 63 B
build/block-library/blocks/list/style.css 63 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 488 B
build/block-library/blocks/media-text/style.css 485 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 474 B
build/block-library/blocks/navigation-link/editor.css 474 B
build/block-library/blocks/navigation-link/style-rtl.css 94 B
build/block-library/blocks/navigation-link/style.css 94 B
build/block-library/blocks/navigation/editor-rtl.css 1.69 kB
build/block-library/blocks/navigation/editor.css 1.69 kB
build/block-library/blocks/navigation/style-rtl.css 1.65 kB
build/block-library/blocks/navigation/style.css 1.64 kB
build/block-library/blocks/navigation/view.min.js 2.52 kB
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 310 B
build/block-library/blocks/page-list/editor.css 310 B
build/block-library/blocks/page-list/style-rtl.css 242 B
build/block-library/blocks/page-list/style.css 242 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 248 B
build/block-library/blocks/paragraph/style.css 248 B
build/block-library/blocks/post-author/editor-rtl.css 210 B
build/block-library/blocks/post-author/editor.css 210 B
build/block-library/blocks/post-author/style-rtl.css 182 B
build/block-library/blocks/post-author/style.css 181 B
build/block-library/blocks/post-comments-form/style-rtl.css 140 B
build/block-library/blocks/post-comments-form/style.css 140 B
build/block-library/blocks/post-comments/style-rtl.css 360 B
build/block-library/blocks/post-comments/style.css 359 B
build/block-library/blocks/post-content/editor-rtl.css 138 B
build/block-library/blocks/post-content/editor.css 138 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/style-rtl.css 143 B
build/block-library/blocks/post-featured-image/style.css 143 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 378 B
build/block-library/blocks/post-template/style.css 379 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 60 B
build/block-library/blocks/post-title/style.css 60 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 361 B
build/block-library/blocks/pullquote/style.css 360 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B
build/block-library/blocks/query-pagination/editor.css 262 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B
build/block-library/blocks/query-pagination/style.css 168 B
build/block-library/blocks/query-title/editor-rtl.css 85 B
build/block-library/blocks/query-title/editor.css 85 B
build/block-library/blocks/query/editor-rtl.css 131 B
build/block-library/blocks/query/editor.css 132 B
build/block-library/blocks/quote/style-rtl.css 169 B
build/block-library/blocks/quote/style.css 169 B
build/block-library/blocks/quote/theme-rtl.css 220 B
build/block-library/blocks/quote/theme.css 222 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 374 B
build/block-library/blocks/search/style.css 375 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 99 B
build/block-library/blocks/separator/editor.css 99 B
build/block-library/blocks/separator/style-rtl.css 250 B
build/block-library/blocks/separator/style.css 250 B
build/block-library/blocks/separator/theme-rtl.css 172 B
build/block-library/blocks/separator/theme.css 172 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 462 B
build/block-library/blocks/site-logo/editor.css 464 B
build/block-library/blocks/site-logo/style-rtl.css 153 B
build/block-library/blocks/site-logo/style.css 153 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 165 B
build/block-library/blocks/social-link/editor.css 165 B
build/block-library/blocks/social-links/editor-rtl.css 812 B
build/block-library/blocks/social-links/editor.css 811 B
build/block-library/blocks/social-links/style-rtl.css 1.33 kB
build/block-library/blocks/social-links/style.css 1.33 kB
build/block-library/blocks/spacer/editor-rtl.css 307 B
build/block-library/blocks/spacer/editor.css 307 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 471 B
build/block-library/blocks/table/editor.css 472 B
build/block-library/blocks/table/style-rtl.css 481 B
build/block-library/blocks/table/style.css 481 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/tag-cloud/style-rtl.css 146 B
build/block-library/blocks/tag-cloud/style.css 146 B
build/block-library/blocks/template-part/editor-rtl.css 636 B
build/block-library/blocks/template-part/editor.css 635 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/term-description/editor-rtl.css 90 B
build/block-library/blocks/term-description/editor.css 90 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 571 B
build/block-library/blocks/video/editor.css 572 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/reset-rtl.css 527 B
build/block-library/reset.css 527 B
build/block-library/theme-rtl.css 688 B
build/block-library/theme.css 692 B
build/block-serialization-default-parser/index.min.js 1.09 kB
build/block-serialization-spec-parser/index.min.js 2.79 kB
build/components/style-rtl.css 15.7 kB
build/components/style.css 15.8 kB
build/compose/index.min.js 10.2 kB
build/core-data/index.min.js 12.3 kB
build/customize-widgets/index.min.js 10.4 kB
build/customize-widgets/style-rtl.css 1.5 kB
build/customize-widgets/style.css 1.49 kB
build/data-controls/index.min.js 614 B
build/date/index.min.js 31.5 kB
build/deprecated/index.min.js 428 B
build/dom-ready/index.min.js 304 B
build/dom/index.min.js 4.53 kB
build/edit-post/classic-rtl.css 492 B
build/edit-post/classic.css 494 B
build/editor/style-rtl.css 3.92 kB
build/editor/style.css 3.91 kB
build/escape-html/index.min.js 517 B
build/format-library/index.min.js 5.36 kB
build/format-library/style-rtl.css 668 B
build/format-library/style.css 669 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 424 B
build/i18n/index.min.js 3.59 kB
build/is-shallow-equal/index.min.js 501 B
build/keyboard-shortcuts/index.min.js 1.49 kB
build/keycodes/index.min.js 1.25 kB
build/list-reusable-blocks/index.min.js 1.85 kB
build/list-reusable-blocks/style-rtl.css 838 B
build/list-reusable-blocks/style.css 838 B
build/media-utils/index.min.js 2.88 kB
build/notices/index.min.js 845 B
build/nux/index.min.js 2.03 kB
build/nux/style-rtl.css 747 B
build/nux/style.css 743 B
build/plugins/index.min.js 1.83 kB
build/primitives/index.min.js 921 B
build/priority-queue/index.min.js 582 B
build/react-i18n/index.min.js 671 B
build/redux-routine/index.min.js 2.63 kB
build/reusable-blocks/index.min.js 2.28 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/server-side-render/index.min.js 1.32 kB
build/shortcode/index.min.js 1.48 kB
build/token-list/index.min.js 562 B
build/url/index.min.js 1.72 kB
build/viewport/index.min.js 1.02 kB
build/warning/index.min.js 248 B
build/widgets/index.min.js 6.27 kB
build/widgets/style-rtl.css 1.04 kB
build/widgets/style.css 1.04 kB
build/wordcount/index.min.js 1.04 kB

compressed-size-action

@ntsekouras

This comment has been hidden.

@glendaviesnz

This comment has been hidden.

@glendaviesnz

This comment has been hidden.

@glendaviesnz glendaviesnz marked this pull request as ready for review Oct 9, 2020
@glendaviesnz glendaviesnz requested review from ajlende and mkevins as code owners Oct 9, 2020
@glendaviesnz

This comment has been hidden.

@simison

This comment has been hidden.

@simison

This comment has been hidden.

@jasmussen

This comment has been hidden.

@ZebulanStanphill

This comment has been hidden.

@glendaviesnz glendaviesnz force-pushed the refactor/gallery-to-nested-image-blocks branch from b538a6b to 0c1daa3 Oct 12, 2020
@paaljoachim

This comment has been minimized.

@glendaviesnz

This comment has been hidden.

@glendaviesnz glendaviesnz force-pushed the refactor/gallery-to-nested-image-blocks branch from 47c2d61 to 4525c2b Oct 13, 2020
@paaljoachim

This comment has been hidden.

@jasmussen

This comment has been hidden.

@glendaviesnz

This comment has been hidden.

@glendaviesnz

This comment has been hidden.

@glendaviesnz

This comment has been hidden.

@glendaviesnz

This comment has been hidden.

@glendaviesnz
Copy link
Contributor Author

@glendaviesnz glendaviesnz commented Jul 30, 2021

Ah ok. I think it'd be fine to do that now, things look good from my perspective.

@antonis Let me know when you have the conflicts on the mobile PR sorted and are ready to merge it back into this PR

mkevins and others added 2 commits Aug 3, 2021
…refactor PR(#31306)

* Move native v1 Gallery components to v1 directory
* Use v1 defaultColumnsNumber in native v1 gallery
* Make onFocus optional in MediaPlaceholder
* Add useInnerBlocksProps hook
* Enable __experimentalGalleryRefactor flag under __DEV__
* Add numColumns to block-list flat list
* Fix spacing
* Adjust styles to avoid appender overlap
* Add gallery caption
* Fix lint
Some of these "fixes" are simply disabling lint for the offending lines.
These currently unused variables may be used in a later PR, so I'm
leaving them in, for now, to help simplify reconciling the changes from
the former refactor PR.
* Fix sass lint
* [Mobile] - Refactor gallery - cherry pick image edit native (#31826)
* Remove duplicate / conflicting methods from performance refactor
* Use context for imageCrop and isGrouped instead of isGallery
* Remove non-existent inheritedAttributes attribute
* Remove dead code from non-existent context attributes
* Remove unused attributes prop from link settings
* Cherry-pick BlockListItem changes
Note: Since render was changed to renderContent, we should return early
from render too, when blockWidth is falsey.
* Return early from render instead of renderContent
* Cherry-pick plumb blockProps through BlockListBlock
I'm not sure yet whether it still makes sense to use blockProps in this
way. I'm going to cherry-pick the commits by file, and sort out the need
for this mechanism afterwards.
* Cherry-pick add GridItem
Since this is duplicated from the original mobile gallery code (Tiles
component), it might make sense to export it for re-use. Previously, it
was only moved, but now that we will maintain both versions, it has
become a duplicate implementation. I will defer this to a later commit.
* Cherry-pick BlockList
Similar to blockProps mentioned earlier, gridProperties will be
evaluated after cherry-picking the relevant changes, to see if there is
another mechanism that may be more appropriate.
* Cherry-pick StylePreview key change
* Cherry-pick blockProps and gridProperties in InnerBlocks
* Use sass var for galleryAppender padding
Note: This also re-adds fullWidth style, which is still being used in
both v1 and v2 mobile implementations. If this is superceded by a recent
refactor of the block width styles, it may be worth revisiting this and
removing / changing the implementation.
* Cherry-pick remaining gallery changes
Note: as before, blockProps and gridProperties should be re-evaluated in
subsequent commits, if necessary. E.g. `imageCrop` is already recieved
via context, and `isGroup` may be sufficient, eliminating the need for
`isGallery`.
* Remove numColumns
Going back over the older commits, it seems there were a two strategies
used to render the gallery images as a grid. One used the numColumns
(same as used in the Columns block), and the other using the Grid
component. This commit cleans up the unused parts of the former
approach.
* Remove blockProps
This is no longer necessary, since we are using context to pass
gallery-level attributes to the image blocks' rendering.
* Fix gallery block.json (delete extra comma)
* Remove unused imageCrop
* Add back blockWidth contentContainerStyles in block list
* Use boolean flags for variants in Platform module
These flags allow for a slightly more flexible, performant, and terse
way of branching by platform. For more details, see:
#18058 (comment)
* Use boolean Platform flags
* Only render imageSizeOptions loading spinner on web
* Add default for destructured context in image edit
This is necessary for unit tests, because they instantiate the block's
edit component directly, and so the default context is not provided.
* Temporarily hard-code experimenal gallery refactor flag to true
This will be reverted once the block settings are fetched from the REST
API. This is enabled for now for testing purposes.
* Update experiments page with warning about the mobile app version
* Changes new gallery flag name
* Updates mobile warning
* Removes the imageCount attribute
* Remove the isGrouped context
* Fixes lint issue

Co-authored-by: Antonis Lilis <[email protected]>
@glendaviesnz
Copy link
Contributor Author

@glendaviesnz glendaviesnz commented Aug 5, 2021

I have looked at what impact this change will have on 3rd party blocks that have a transform->to and and/or transform/from the core/gallery.

The transform->to in particular will be difficult in the short term as I can't see a way from the transform callback to detect if the block should be transformed to the innerBlocks version or not. So, I think it would make sense to add some filters to temporarily handle the 3rd block block transformations to give plugin developers time to update code - which may require innerBlocks version to be in a core release to allow detection of which version to transform to.

I have a PR which has some filters added to handle this, which seems to work when used with Jetpack and CoBlocks gallery blocks. @talldan, @antonis do you have any thoughts on this?

Given the size of this PR, if anyone has any thoughts it may be easier to discuss over at this PR.

…d from 3rd party blocks (#33861)

Co-authored-by: Glen Davies <[email protected]>
Co-authored-by: Andrew Serong <[email protected]>
Glen Davies added 2 commits Aug 16, 2021
…gallery-to-nested-image-blocks

# Conflicts:
#	packages/block-library/src/image/image.js
#	packages/block-library/src/image/styles.native.scss
This is necessary for rendering cropped / uncropped images on mobile,
see: #31826 but was
inadvertantly removed (possibly during a merge?), so this commit is just
adding it back.
@talldan
Copy link
Contributor

@talldan talldan commented Aug 17, 2021

@glendaviesnz How's this one going? Any issues remaining to address? I see there are still some commits being made.

@glendaviesnz
Copy link
Contributor Author

@glendaviesnz glendaviesnz commented Aug 18, 2021

@glendaviesnz How's this one going? Any issues remaining to address? I see there are still some commits being made.

@talldan a couple of minor issues were discovered in final testing, so commits have been pushed for those. We are close to finishing up the testing, and just one final issue I am looking into to see if it needs to be fixed before a merge or not.

The other issues have been prioritised on the Gallery project board, with the other remaining ones able to be follow up PRs I think - but let me know if you think any of them should be looked at prior to a merge.

I have also added a draft dev note on make/core if you want to review that.

Will let you know when I have looked at that final issue. Let me know if you think there is anything else that needs following up.

Copy link
Contributor

@talldan talldan left a comment

Seems like everything is ready to go!

Awesome work @glendaviesnz.

@glendaviesnz glendaviesnz merged commit 3e645c4 into trunk Aug 19, 2021
20 checks passed
20 checks passed
@github-actions
Compute current and next stable release branches
Details
@github-actions
test
Details
@github-actions
Check (14)
Details
@github-actions
Checks (12)
Details
@github-actions
Admin - 1 Admin - 1
Details
@github-actions
Run performance tests
Details
@github-actions
pull-request-automation (14)
Details
@github-actions
test (12.2, gutenberg-editor-initial-html, 14)
Details
@github-actions
JavaScript (12)
Details
@github-actions
Checks (14)
Details
@github-actions
Admin - 2 Admin - 2
Details
@github-actions
JavaScript (14)
Details
@github-actions
Admin - 3 Admin - 3
Details
@github-actions
Admin - 4 Admin - 4
Details
@github-actions
Bump version
Details
@github-actions
Build Release Artifact
Details
@github-actions
Mobile
Details
@github-actions
Create Release Draft and Attach Asset
Details
@glendaviesnz glendaviesnz deleted the refactor/gallery-to-nested-image-blocks branch Aug 19, 2021
@github-actions github-actions bot added this to the Gutenberg 11.4 milestone Aug 19, 2021
antonis added a commit that referenced this pull request Sep 2, 2021
* Refactor gallery to nested image blocks

* Fix issue with images not loading on first selection from media gallery

* Remove the default columns setting as we don't have access to innerBlocks at the point that the block validation is run

* Revert "Remove the default columns setting as we don't have access to innerBlocks at the point that the block validation is run"

This reverts commit 4d95e95.

* Add image count so we can work out default columns as innerBlocks not available at point of block validation

* Disable the innerBlocks dropzones so drag drop works same as existing gallery - with the idea that we will improve the innerblocks drag and drop in a later PR

* Lint changes

* Revert "Lint changes"

This reverts commit 9b0abcc.

* Revert "Disable the innerBlocks dropzones so drag drop works same as existing gallery - with the idea that we will improve the innerblocks drag and drop in a later PR"

This reverts commit cc05d60.

* Suggested solution for handling multiple file drop into gallery

* Remove non image files from drag and drop and disable individual image drop zone

* Fix transform to individual images

* Fix transform from individual images

* Revert drag and drop transform changes

* Add gallery transform to image block to override the default gallery transform when dragging multiple images onto the gallery

* Move handling of file uploads to Gallery from media placeholder

* split innerblocks mapping into separate effect to reduce chatter

* Add useMemo to currentImageOptions

* reuse existing innerBlocks rather than recreating with every new image selection

* Switch to useMemo for updating local image const instead of local component state

* Fix issue with image sizing not being available on initial load of component some times

* Memoise the useImageSizes hook

* Fix issue with media browser  defaulting to edit gallery view

* Fix missed incorrect use of addToGallery

* Add some extra effects for getting the imageData as the getMedia call is async so need to keep circling through the innerblocks updates until we have all the data we need

* Simplify the imageData by using a useSelect

* Another optimisation - only return a new imageData reference if all images have data resolved

* Refactored Gallery: Add loading state to gallery image size options (#27087)

* Add loading spinner for image size options
Co-authored-by: Glen Davies <[email protected]>

* Initial deprecations commit

* Fix issue with linkDestination not being applied in migration

* Refactor gallery deprecations

* Fix missing attributes from migration

* Update deprecation to set allowResize

The imageEdit component defaults the context value for allowResize to true. The refactored gallery sets this to false by default. Setting allowResize to false when migrating a deprecated gallery allows images to be cropped in the display the same as the gallery when the post is saved and gallery reloaded.

* Fix issue with crop not working when certain plugins are loaded

* Fix SCSS lint errors

* Update the block example

* Linting fixes

* Fix the e2e test and the accessibility issue with having aria group role on a list item

* Fix the e2e test and the accessibility issue with having aria group role on a list item

* Fix frontend omission of wp-block-image class

Also tweaks the gallery styles to remove the margins the use of `wp-block-image` introduces when it is moved to the inner `figure` element.

* change deprecation to use imageCount as isEligible check as it seems that some previous block versions may not have an ids attribute

* Move back to a single deprecations file and reorder in array

* Remove additional check in v5 isEligible

* Fix the v4 migration

* Fix styles for Safari compatibility

* Remove unnecessary gallery editor styles

* Fix typo in deprecations

* Restore styles to render deprecated gallery versions

* Avoid applying flex styles to IE11

* Add additional selector to prevent the hidden individual image drop zones ending up display flex instead of hidden

* IE11 styling improvements

* Apply default style class to new images added to gallery

* fix linting issues

* Move block props to the outer wrapper

* Revert "Move block props to the outer wrapper"

This reverts commit e0723a7.

* Revert "Revert "Move block props to the outer wrapper""

This reverts commit a7504fd.

* Fetch media if isListItem is true

* Change context from isListItem to isGrouped

* Remove wrapper div

* remove extra wrapper around media placeholder and caption and use flex css instead

* Revert "remove extra wrapper around media placeholder and caption and use flex css instead

* Revert "Remove external div wrapper by moving InnerBlocks to a fragment"

* another update to image wrapper

* put media uploader outside figure so structure matches front end

* Replace div with View for the sake of native code

* Move setting of attributes to the child images

* Gallery Block Refactor: Account for null image ids in gallery migrations (#27855)

Co-authored-by: Glen Davies <[email protected]>

* Remove the gradient and put caption under image if is-rounded style applied (#27869)

* Add alignment fixes for non cropped images

Co-authored-by: Glen Davies <[email protected]>

* Remove outer div wrapper

* Keep image margins while dragging sibling

* Fix e2e test expected markup to match new structure

* Gallery Block Refactor: Add handling of short code transforms

* Removed unused prop

* Account for undefined block and innerblocks in conversion to reusable block lifecycle

* Add custom gutter sizes to refactored gallery (#28377)

* Adjust editor styles to match new dom structure

* Remove redundant styles that are duplicated in nested image blocks

* Fix issue with Image block dragged out of Gallery still having inheritedAttributes set

* When dragging an image block into a gallery make sure we don't wipe any custom links

* fix issue with variable declaration order

* Fix bug with custom link being overwritten by gallery linkTo changes

* Fix application of gutter size CSS var (#28759)

* Fix mobile width for gallery images

* Add missing dependency to inner images selector

* remove conversion to cover block if image in gallery

* Add fallback to old gallery edit and save for existing gallery (#28961)

Co-authored-by: Glen Davies <[email protected]>

* Remove duplicate import

* Remove need for temporary imageUploads attribute as we can just create the innerBlocks as part of transform

* Remove handling of gallery attribute updates from child images

* Move updating of attributes back to gallery and show snackbar to indicated to user that change was made to all images in the gallery

* Update transforms to work with both versions of gallery

* Remove redundant changes

* Remove redundant changes

* Remove unused method

* Merge similar Image transforms.

* Fix some issues missed when moving attribute setting back to gallery.

* Simplify handling of images from media placeholder now that drag and drop files handled by transform

* Fix typo

* Fix broken upload from media placeholder

* Fix issue with new file uploads overwriting existing blocks

* Remove isGrouped dependency from call to get image as not needed now attributes handled by gallery

* Remove custom gutter size feature

Reasoning for removal can be found at #25940 (comment)

* Use getMediaItems instead of getMedia for getting image data for all gallery images

* Add back missing context prop from rebase conflict

* Make use of new uniqueByBlock flag on updateBlockAttributes to batch child image updates and allow single undo to revert

* Move the setting of the default attributes on new blocks to a single useEffect (#29328)

Co-authored-by: Glen Davies <[email protected]>

* Add snackbar notice ids (#29364)

* Account for people adding and  removing images from media browser so number of images the same, but list of images different to current.

* Changes from PR feedback

* Fix issue with deprecated constants

* Fix linting and e2e test failures

* Some more e2e fixes

* Add transform from old gallery to new format

* Memoize the allowedBlocks

This allows the shallow equals check in the inner blocks' `useNestedSettingsUpdate` hook to pass successfully preventing the state update loop.

* Add warning about image formats required if uploading to gallery

* Move allowedBlocks outside of component to avoid useMemo use

* Re-apply uncropped alignment changes lost in rebase

* Fix issue with non-cropped images in deprecated gallery

* Don't show media library buttons while images are still uploading as the media library needs the image ids to reconcile which images are already in the gallery

* Remove gallerRef that was no longer doing anything

* Respect sort order from Media Library (#30070)

Co-authored-by: Glen Davies <[email protected]>

* Gallery block refactor: make invalid file type errors consistent (#30396)


Co-authored-by: Glen Davies <[email protected]>
Co-authored-by: Aaron Robertshaw <[email protected]>

* Fix issue with invalid type message when adding files via media browser 'Upload' button

* Gallery block refactor: check for new images by clientId instead of id to stop link settings being lost when images edited (#30550)

Co-authored-by: Glen Davies <[email protected]>

* Apply changes from rebase to deprecated gallery

* Apply patch from rebase to not select all items in media library when existing images have no ids

* Copy caption across from image selected from media library (#30784)

Co-authored-by: Glen Davies <[email protected]>

* Add data-id to image to help with compatibility of refactored gallery with existing themes and plugins (#30710)

Co-authored-by: Glen Davies <[email protected]>

* Revert "Add data-id to image to help with compatibility of refactored gallery with existing themes and plugins (#30710)"

This reverts commit a2253ec.

* fix bug with image style being lost when gallery grouped (#31068)

Co-authored-by: Glen Davies <[email protected]>

* Only add RichText component if the figcaption is clicked to prevent it stealing focus every time block is selected (#31216)

Co-authored-by: Glen Davies <[email protected]>

* Move native v1 Gallery components to v1 directory

* Use v1 defaultColumnsNumber in native v1 gallery

* Fix bug with alt text not being copied from media library (#31066)

Co-authored-by: Glen Davies <[email protected]>

* Make onFocus optional in MediaPlaceholder

* Add useInnerBlocksProps hook

* Enable __experimentalGalleryRefactor flag under __DEV__

This is only temporary, for testing purposes. This will be replaced with
an actual implementation (which will need to access the flag remotely).

* Add WIP v2 gallery

* Add numColumns to block-list flat list

* Temporarily comment out spinner for pending imagesize option

This will need to be addressed (since we can't have unwrapped text on
mobile). For now, I'm commenting this out to proceed with cherry-picking
the changes from the original mobile refactor branch.

* Fix spacing

* Adjust styles to avoid appender overlap

* Add gallery caption

* Fix lint

Some of these "fixes" are simply disabling lint for the offending lines.
These currently unused variables may be used in a later PR, so I'm
leaving them in, for now, to help simplify reconciling the changes from
the former refactor PR.

* Fix sass lint

* [Mobile] - Refactor gallery - cherry pick image edit native (#31826)

* WIP-commit bring image changes from final state of original mobile PR

This has unresolved / unmarked conflicts which will be resolve in
subsequent commits. I am separating the commit to make the resolution
process more transparent.

* Remove duplicate / conflicting methods from performance refactor

* Use context for imageCrop and isGrouped instead of isGallery

* Remove non-existent inheritedAttributes attribute

* Remove dead code from non-existent context attributes

* Remove unused attributes prop from link settings

* Cherry-pick BlockListItem changes

Note: Since render was changed to renderContent, we should return early
from render too, when blockWidth is falsey.

* Return early from render instead of renderContent

* Cherry-pick plumb blockProps through BlockListBlock

I'm not sure yet whether it still makes sense to use blockProps in this
way. I'm going to cherry-pick the commits by file, and sort out the need
for this mechanism afterwards.

* Cherry-pick add GridItem

Since this is duplicated from the original mobile gallery code (Tiles
component), it might make sense to export it for re-use. Previously, it
was only moved, but now that we will maintain both versions, it has
become a duplicate implementation. I will defer this to a later commit.

* Cherry-pick BlockList

Similar to blockProps mentioned earlier, gridProperties will be
evaluated after cherry-picking the relevant changes, to see if there is
another mechanism that may be more appropriate.

* Cherry-pick StylePreview key change

* Cherry-pick blockProps and gridProperties in InnerBlocks

* Use sass var for galleryAppender padding

Note: This also re-adds fullWidth style, which is still being used in
both v1 and v2 mobile implementations. If this is superceded by a recent
refactor of the block width styles, it may be worth revisiting this and
removing / changing the implementation.

* Cherry-pick remaining gallery changes

Note: as before, blockProps and gridProperties should be re-evaluated in
subsequent commits, if necessary. E.g. `imageCrop` is already recieved
via context, and `isGroup` may be sufficient, eliminating the need for
`isGallery`.

* Remove numColumns

Going back over the older commits, it seems there were a two strategies
used to render the gallery images as a grid. One used the numColumns
(same as used in the Columns block), and the other using the Grid
component. This commit cleans up the unused parts of the former
approach.

* Remove blockProps

This is no longer necessary, since we are using context to pass
gallery-level attributes to the image blocks' rendering.

* Fix gallery block.json (delete extra comma)

* Remove unused imageCrop

* Gallery refactor - Infer version from existing content (#32270)

* Give content precedence over flag in edit wrapper

This modifies the logic in the gallery edit wrapper function to infer
version information from existing content when the editor encounters
content created from the new implementation and the flag is not set.

* Resolve merge conflicts

This brings the gallery edit changes to the v1 directory, and merges the
changes for the image block.

* Use non-deprecated hook when possible

This brings the same change from this PR:
#31027 which may have missed
the deprecated hook in the refactor PR, since it hadn't landed yet.

* Resolve conflict in block editor default settings

This was an update for a new jsdoc linting rule.

* Add back blockWidth contentContainerStyles in block list

I believe this was inadvertantly removed in some earlier commits, so
this commit adds it back.

* Use boolean flags for variants in Platform module

These flags allow for a slightly more flexible, performant, and terse
way of branching by platform. For more details, see:
#18058 (comment)

* Use boolean Platform flags

* Only render imageSizeOptions loading spinner on web

* Add default for destructured context in image edit

This is necessary for unit tests, because they instantiate the block's
edit component directly, and so the default context is not provided.

* Temporarily hard-code experimenal gallery refactor flag to true

This will be reverted once the block settings are fetched from the REST
API. This is enabled for now for testing purposes.

* Revert "Temporarily hard-code experimenal gallery refactor flag to true"

This reverts commit 6fbaed9.

* Update experiments page with warning about the mobile app version

* Resolve conflict in image.js

The conflict was from #33095

* Pass the Gallery v2 Flag over from the editor

* Capture Gallery Refactor in initial props

* Lint Fix

* Minor changes from code review:
* Reward experiments description
* Add documentation about new media placeholder handleUpload flag
* Add additional explanation of v1-6 deprecations
* Rename the edit wrapper components to make useage clearer
* Temporarily remove tranform from v1 gallery to v2 gallery

* Improve naming of save and deprecated save methods

* Rename __experimentalGalleryRefactor flag to __unstableGalleryWithInnerBlocks

* Remove the isGrouped context as no longer needed.

* Rename __unstableGalleryWithInnerBlocks to _unstableGalleryWithImageBlocks

* Gallery block refactor: remove the imageCount attribute  (#33677)

* Remove the imageCount attribute and use CSS instead to set default columns

Co-authored-by: Glen Davies <[email protected]>
Co-authored-by: Aaron Robertshaw <[email protected]>

* Fix broken scss

* Fix php linting error

* Changes new gallery flag name

* Updates mobile warning

* Removes the imageCount attribute

* Remove the isGrouped context

* Fixes lint issue

* [Mobile] Renames Gallery v2 Flag to __unstableGalleryWithImageBlocks (#33816)

* Renames __experimentalGalleryRefactor to __unstableGalleryWithImageBlocks

* Fixes lint formatting issue

* Fixes lint issue

* Merge mobile refactor of gallery to nested image blocks into desktop refactor PR(#31306)

* Move native v1 Gallery components to v1 directory
* Use v1 defaultColumnsNumber in native v1 gallery
* Make onFocus optional in MediaPlaceholder
* Add useInnerBlocksProps hook
* Enable __experimentalGalleryRefactor flag under __DEV__
* Add numColumns to block-list flat list
* Fix spacing
* Adjust styles to avoid appender overlap
* Add gallery caption
* Fix lint
Some of these "fixes" are simply disabling lint for the offending lines.
These currently unused variables may be used in a later PR, so I'm
leaving them in, for now, to help simplify reconciling the changes from
the former refactor PR.
* Fix sass lint
* [Mobile] - Refactor gallery - cherry pick image edit native (#31826)
* Remove duplicate / conflicting methods from performance refactor
* Use context for imageCrop and isGrouped instead of isGallery
* Remove non-existent inheritedAttributes attribute
* Remove dead code from non-existent context attributes
* Remove unused attributes prop from link settings
* Cherry-pick BlockListItem changes
Note: Since render was changed to renderContent, we should return early
from render too, when blockWidth is falsey.
* Return early from render instead of renderContent
* Cherry-pick plumb blockProps through BlockListBlock
I'm not sure yet whether it still makes sense to use blockProps in this
way. I'm going to cherry-pick the commits by file, and sort out the need
for this mechanism afterwards.
* Cherry-pick add GridItem
Since this is duplicated from the original mobile gallery code (Tiles
component), it might make sense to export it for re-use. Previously, it
was only moved, but now that we will maintain both versions, it has
become a duplicate implementation. I will defer this to a later commit.
* Cherry-pick BlockList
Similar to blockProps mentioned earlier, gridProperties will be
evaluated after cherry-picking the relevant changes, to see if there is
another mechanism that may be more appropriate.
* Cherry-pick StylePreview key change
* Cherry-pick blockProps and gridProperties in InnerBlocks
* Use sass var for galleryAppender padding
Note: This also re-adds fullWidth style, which is still being used in
both v1 and v2 mobile implementations. If this is superceded by a recent
refactor of the block width styles, it may be worth revisiting this and
removing / changing the implementation.
* Cherry-pick remaining gallery changes
Note: as before, blockProps and gridProperties should be re-evaluated in
subsequent commits, if necessary. E.g. `imageCrop` is already recieved
via context, and `isGroup` may be sufficient, eliminating the need for
`isGallery`.
* Remove numColumns
Going back over the older commits, it seems there were a two strategies
used to render the gallery images as a grid. One used the numColumns
(same as used in the Columns block), and the other using the Grid
component. This commit cleans up the unused parts of the former
approach.
* Remove blockProps
This is no longer necessary, since we are using context to pass
gallery-level attributes to the image blocks' rendering.
* Fix gallery block.json (delete extra comma)
* Remove unused imageCrop
* Add back blockWidth contentContainerStyles in block list
* Use boolean flags for variants in Platform module
These flags allow for a slightly more flexible, performant, and terse
way of branching by platform. For more details, see:
#18058 (comment)
* Use boolean Platform flags
* Only render imageSizeOptions loading spinner on web
* Add default for destructured context in image edit
This is necessary for unit tests, because they instantiate the block's
edit component directly, and so the default context is not provided.
* Temporarily hard-code experimenal gallery refactor flag to true
This will be reverted once the block settings are fetched from the REST
API. This is enabled for now for testing purposes.
* Update experiments page with warning about the mobile app version
* Changes new gallery flag name
* Updates mobile warning
* Removes the imageCount attribute
* Remove the isGrouped context
* Fixes lint issue

Co-authored-by: Antonis Lilis <[email protected]>

* [Mobile] Android: Pass the Gallery v2 Flag over from the editor (#33544)

Pass the Gallery v2 Flag over from the editor on Android

Co-authored-by: Glen Davies <[email protected]>
Co-authored-by: Aaron Robertshaw <[email protected]>
Co-authored-by: Glen Davies <[email protected]>
Co-authored-by: Matthew Kevins <[email protected]>
Co-authored-by: Matthew Kevins <[email protected]>
Co-authored-by: Antonis Lilis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment