Refactored Gallery: Add filter for replacing inner gallery component for alternate layouts #29244
Conversation
…ocks at the point that the block validation is run
… innerBlocks at the point that the block validation is run" This reverts commit 4d95e95.
… available at point of block validation
… gallery - with the idea that we will improve the innerblocks drag and drop in a later PR
This reverts commit 9b0abcc.
…existing gallery - with the idea that we will improve the innerblocks drag and drop in a later PR" This reverts commit cc05d60.
…transform when dragging multiple images onto the gallery
…mponent some times
… is async so need to keep circling through the innerblocks updates until we have all the data we need
…mages have data resolved
…27087) * Add loading spinner for image size options Co-authored-by: Glen Davies <[email protected]>
aaronrobertshaw
added
[Block] Gallery
|
Size Change: +21 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
gziolo
added
the
[Feature] Extensibility
| @@ -92,4 +92,4 @@ function RichTextVisibilityHelper( { isHidden, ...richTextProps } ) { | |||
| ); | |||
| } | |||
|
|
|||
| export default Gallery; | |||
| export default withFilters( 'editor.Gallery' )( Gallery ); | |||
There was a problem hiding this comment.
We've been moving away from this kind of filters in Gutenberg in general. These show too much internal implementation details and it locks the blocks (we can't iterate on them in the future).
In general, we do not favor filters and hooks in blocks, we prefer if third-party developpers rely one the Block APIs instead.
If the intent is to change the "Edit" function entirely like this seems to do, I think it's better if it's a separate block with a "transform" back and forth. (if it's not possible to implement using a block style variation)
There was a problem hiding this comment.
Thanks for the quick feedback on this, appreciate it!
The desire here is to avoid separate blocks completely and to make the switching of gallery layouts as simple as selecting a block style. This makes discovering the different gallery layouts easy, encourages experimentation etc. The different gallery layouts aren't "hidden" as block transforms and the user isn't overwhelmed by multiple gallery blocks in the inserter.
So the intent is to only change the layout of the inner image blocks within the Gallery not the entire edit function.
To continue the masonry example from the PR description, a masonry layout would require a little JS or wrapping component along with some CSS within the editor. Yet, other than wrapping the inner image blocks in a masonry component, it might not need to touch anything else. Duplicating the entire GalleryEdit component to achieve this would be fragile and feels like overkill while multiplying the maintenance burden. I thought adding a masonry block style directly to the core gallery would also be outside its scope.
I can appreciate the concerns regarding possibly locking the block from future iterations. Do you have any suggestions regarding how we might achieve the above goals leveraging the current state of the Block API? Or even if it would be worth expanding the API to allow something like this wrapping rendering of inner blocks?
There was a problem hiding this comment.
The desire here is to avoid separate blocks completely and to make the switching of gallery layouts as simple as selecting a block style.
I can understand that, though, it's a difficult balance and for me, the cons for having this filter are bigger than the pros especially since we have alternatives (even if less compelling). Developers are going to abuse this kind of filters making the block harder to maintain and evolve in the future.
There was a problem hiding this comment.
Developers are going to abuse this kind of filters making the block harder to maintain and evolve in the future.
That's a good point. I'll leave this PR open temporarily on the off chance someone else has a creative idea on how we can achieve the desired result without the downsides.
There was a problem hiding this comment.
In general, current "block styles" are a poor and opaque mechanism that — while being simple to adopt — has shown to have many shortcoming. For one, the editor knows nothing about style variations outside of a "class". A style is not described through attributes (the properties of blocks) which means logic is always going to be a fork, implementation wise, for any of its interfaces (edit / save). In that sense, style variations should be limited to simple cosmetic choices that carry no functional changes.
I'd expect masonry to also possibly have some configuration added to it in the future, which couldn't be expressed through a style variation.
To me the right way to achieve the initial goal would be to make InnerBlocks capable of masonry on its own, for whatever content is included within. Then a masonry gallery (after the refactor to use child image blocks) is just a gallery block variation that sets inner blocks to use masonry. That would also allow to have masonry applied to lists of posts on query patterns, for example.
The one thing styles have going on for them is prominence in the UI. But that can easily be absorbed by making proper block variations more canonical, or defining a block variation as a style, which would structurally be more resilient.
There was a problem hiding this comment.
Thanks @mtias for the feedback and interesting ideas.
To me the right way to achieve the initial goal would be to make InnerBlocks capable of masonry on its own, for whatever content is included within.
Essentially, the aim of the filter was to allow for the InnerBlocks children to be laid out as desired. In the masonry case, to conditionally wrap the InnerBlocks children in a masonry component if the style was selected.
If InnerBlocks was capable of having a layout provider passed to it depending on a gallery attribute that would make the block variation option more viable.
I'm wondering how flexible that can be made to cater for other possible layouts such as tiled, mosaic etc.
The one thing styles have going on for them is prominence in the UI. But that can easily be absorbed by making proper block variations more canonical, or defining a block variation as a style, which would structurally be more resilient.
Combining this idea with the first sounds like it would tick a lot of boxes.
There was a problem hiding this comment.
To add more context, there's a proposal for an independent gallery with a masonry layout, which as said in this thread could transform from-to the main gallery block and have its own options and HTML structure, which is more extensible and maintainable approach than using block styles.
There was a problem hiding this comment.
Hi @priethor, thanks for the extra info.
The real aim we were trying to achieve is simplifying the user experience around galleries.
I agree an independent block has its benefits. However, once a site has the core gallery, masonry, tiled, mosaic and more layouts for a gallery, the user now needs to know they exist in the block inserter to try them, understand that they can switch between them via the block transforms and so on. On top of this, if they are all separate attempting to keep their functionality and features up-to-date across all galleries is more problematic.
If it is possible for InnerBlocks to be updated to handle different layouts, this could still cater to the masonry layout
proposed in #28247
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
from
ff1c4af to
90b1aff
Compare
aaronrobertshaw
requested review from
ajitbohra,
ajlende,
ellatrix,
nerrad,
ntwb,
spacedmonkey and
TimothyBJacobs
as code owners
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
2 times, most recently
from
1925280 to
418ecc8
Compare
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
2 times, most recently
from
7dbc807 to
b9ae71a
Compare
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
2 times, most recently
from
f4d0ea6 to
9415d57
Compare
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
from
66bdda6 to
4e57fb3
Compare
|
Closing this PR as the filter approach has been decided against. |
Description
Adds a filter to allow replacing of the inner Gallery component. This will provide a means for plugins to alter the gallery's layout within the editor without having to completely reimplement the
GalleryEditcomponent using theeditor.BlockEditfilter.A prime example of this would be a masonry layout using a dedicated React masonry component to lay out the inner blocks.
Why add this filter?
To a user, a different gallery layout is for all intents and purposes, just a style of gallery. If different gallery layouts can be presented as block styles, it will aid discovery and user experimentation as well as simplify the options in the block inserter.
The catch here is block styles alone will not help us manage the gallery's layout within the editor. In the above example of a masonry layout, we can get away with adding a plugin to enqueue frontend assets. Unfortunately, we need more control within the editor.
The GalleryEdit component is rather complex and presents a barrier to being able to simply swap it out for an alternate via the
editor.BlockEditfilter. This new filter narrows the scope of the component being replaced making it much easier to augment the gallery.How has this been tested?
Manually.
Something along the lines of:
Screenshots
Types of changes
New feature
Checklist: