Gallery: Support for ID-less shortcode

[mcsf]

Per the WP Codex, the [gallery] shortcode should be supported when no IDs are provided:

The default behavior, if no ID is specified, is to display images attached to the current post.

— https://codex.wordpress.org/Gallery_Shortcode

Description

Pasting [gallery] with no extra attributes, namely ids, generates a Gallery block with the post's attached media. Same goes for converting from classic post to blocks.

Approach

1. A new attribute, useAttachedImages, is added to Gallery. It defaults to false.
2. The shortcode transform for Gallery recognizes when no IDs are provided, in which case useAttachedImages is set to true for the generated Gallery block. (This transform kicks in when pasting shortcodes, but also when converting classic content to blocks.)
3. If a Gallery block has useAttachedImages as true, request information on the images attached to the current post from the core media endpoint. This request is performed via withAPIData.
4. Once the data is ready, write it to the block's attributes, after which the block behaves as a regular image-provided Gallery.

How Has This Been Tested?

1. Create a post with the classic editor.
2. Upload images to the post by dragging and dropping them from your filesystem.
3. Save the post, for good measure.
4. Delete its contents, save again.
5. Open the post with Gutenberg.
6. In the empty Paragraph, paste [gallery].

It should be rendered as a Gallery block comprising the images originally attached to the post.

Screenshots (jpeg or gifs if applicable):

Caveat / To fix

• As I was preparing this summary, I realized that the process described above fails after the first time in a given Gutenberg session, and the block gets stuck "loading" the attached media:

• Allow dynamic behavior (Gallery: Support for ID-less shortcode #3852 (comment))

Types of changes

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows has proper inline documentation.

[mcsf]

One thing to consider is that the conversion process commits image IDs to the block’s attributes. This effectively makes the block static, a departure from the behavior of the original shortcode, which was dynamic. In other words, if some agent somewhere modifies which media items are attached to the post, this Gallery block will not reflect those changes.

The behavior could be made entirely dynamic by not committing the attached images to the attributes (e.g., Latest Posts), but there are trade-offs to make.

• A static gallery may behave more predictably: we get editing controls for free, allowing a user to switch images as for any other gallery. Copy/paste works just the same within a post and across posts.

• However, a static gallery could mean that, in certain situations where the same post is edited with the classic editor and Gutenberg, a user is caught off-guard if the shortcode-turned-block no longer updates on its own based on post attachments.

[mtias]

We don't have a way to create a dynamic gallery from scratch, so at the moment converting an old one should make it static. That said, it might be worth allowing this as an option in the inspector — "load all images attached to this post". Toggling it makes the block save null and be rendered server side. How does that sound?

[mcsf]

Sounds good. If it's small enough I'll fit it in this PR.

[youknowriad]

The data module might help here since it's a canonical way to access the WP client state #3832
But we can't assume the post is present, I'm thinking the gallery block should work for site building later and we don't have a post necessarily. This option should be only available if we detect an "editor" state and a "post Id".

[mcsf]

#3832

I had started to review this and should’ve connected the dots. Thanks, this sounds like a good way to try the module.

can’t assume […] site building […] only if we detect

Yes, great points!

[mcsf]

Rewritten using the newest wp.data, ready for review. I think we can leave the following for another PR:

We don't have a way to create a dynamic gallery from scratch, so at the moment converting an old one should make it static. That said, it might be worth allowing this as an option in the inspector — "load all images attached to this post". Toggling it makes the block save null and be rendered server side. How does that sound?

[danielbachhuber]

Flagging as Needs Design Feedback because of:

We don't have a way to create a dynamic gallery from scratch, so at the moment converting an old one should make it static.

This presents the same scope of problem as described in #4522 (comment)

We shouldn't unexpectedly lose user data in the block conversion process. If Gutenberg wants to deliberately ignore some data, then it will need to make it obvious to the end user (or prevent the conversion from happening.

In this particular case, the current [gallery] transformation will change the user's expectation of the user experience.

I'm not opposed to changing this behavior. I only want to highlight that we need to make it a deliberate, communicated decision. One idea: user-test this change with a couple of users to get their opinion on whether this is a significant enough change to merit more UX.

[mcsf]

Thanks for the perspective, @danielbachhuber. My very personal opinion is that the concept of attaching media to a post is just one of those very weird WordPress intricacies that should, one way or another, be deprecated at some point. By this I mean no disrespect to those using [gallery] or equivalent techniques to display media dynamically: these have all been ways to deal with some of the limitations of WordPress, either at the editing or the backend level.

In a Gutenberg-based future, here's how I see things:

(editing limitations) Some users just wanted a nice way to construct a gallery whose items may change in the future, so they relied on the automatic media attachment feature. For these, a Gallery block hopefully offers a much better experience.

(backend limitations) Some users may be doing things in the backend: trigger changes to attachments based on certain updates, add/remove attachments via the REST API, etc. For these, the editing interface won't cut it, obviously. However, it's fair to say the API will catch up with blocks: I'd expect that we'll be able to treat blocks in a post as resources, whether at the lowest level (direct manipulation of the attributes object and inner HTML) or a block-provided abstraction (for a Gallery: add image, remove image). Until this beautiful day comes, I can understand that we want to support dynamic galleries, though.

I have no strong opinion here. If the way forward is support dynamic behavior, please express yourselves. :) Note that dynamic blocks currently can't handle their inner HTML, only comment attributes (ref), and that kind of support isn't ready for primetime. This means we can't have a block whose static or dynamic character depends on its attributes. Thus, if we want to support dynamic galleries now, the solution I see is creating a new block type just for dynamic galleries. The final UX for all of this would be tricky to get right, though (would we burden the user with the question of whether they want their gallery to be static or dynamic? what does that mean for most?).

[danielbachhuber]

I'd love to hear @westonruter's opinion on this too.

[westonruter]

I'll note that when we added shortcode support to the Text widget one of the key use cases for it was to allow for a Text widget to have a [gallery] and for this to use the attachments from the current post on which the widget is being displayed. This allows for a widget in a sidebar to vary based on the queried post. I can see this as being particularly important when using a gallery as a reusable block. For more see https://core.trac.wordpress.org/ticket/42548

[mcsf]

@westonruter: That's good insight, thanks, especially the expectations around the Text widget.

This allows for a widget in a sidebar to vary based on the queried post. I can see this as being particularly important when using a gallery as a reusable block.

Good point, although, at the same time, how could we avoid creating confusion between the two scenarios:

• As you described, a user creates a reusable arrangement of blocks which includes a gallery. The gallery is set to dynamically use images attached to the post. As the reusable unit is added to different posts, the reusable arrangement will render differently on the front-end, reflecting the post's attached media. The user intended this sort of magical behavior, and the reusable unit could be labeled as "Post's images".
• Alternatively, a user creates a reusable arrangement of blocks which includes a gallery. That post happens to have attached media, and the gallery is shown in the editor with the post's attached images. The user may not realize why this is. As the reusable unit is added to different posts, the user expects to see the same thing across posts, and expects to be able to change media once to update everywhere (that is, in any post that has the reusable unit).

This sort of challenge makes me cautious about trying to fit everything into the core Gallery block. Similar questions: when in "dynamic" ([gallery]) mode, do we let the user transparently manipulate the images in the block, and can those changes be persisted in the post's attachments? This starts to sound like a lot of responsibilities for a single block and makes me contemplate "Dynamic gallery" (for lack of a better name right now) as an alternative — even though this would introduce another point of confusion: "which block should I use?"

[westonruter]

The user intended this sort of magical behavior

You're right. It is a “power user” feature, that is, a hack. But it's a key use case that is depended on.

This starts to sound like a lot of responsibilities for a single block and makes me contemplate "Dynamic gallery" (for lack of a better name right now) as an alternative — even though this would introduce another point of confusion: "which block should I use?"

I think we're getting closer into a site customization context here, where we're getting into territory that is outside of specifically a post context. Clearly the gallery shortcode in this case allows for a variable post context to draw from. This could also be the case for the Text widget generally, as shortcodes used in the Text widget could be referencing postmeta for the current queried post and thus also be variable as the same widget is displayed on different pages.

In site customization, I expect there is going to be a need to identify certain blocks that get re-used across multiple/all templates of a site. For example, there should be header blocks and sidebar blocks that contain nested blocks inside of them. These nested blocks may need to vary depending on which URL they are being rendered on. For instance, a nav menu has the class names render differently depending on the URL that it is rendered on (e.g. so items can be styled as the “current” page or not).

So I think the question (and problem) is somewhat wider than just the gallery. It's the local block vs global block problem.

I don't know of a user-friendly way to present this to the user per se, but what if there was a sort of “HOC” container block for post context? Maybe this could be presented as a sidebar control instead of a block container instead, but there could be an advanced way to specify the context that a specific block should be rendered in. By default it should be the current post being edited. But it could also be set to the current post being viewed. Maybe that idea could be combined with reusable blocks to implement the [gallery] behavior?

[mcsf]

I don't know of a user-friendly way to present this to the user per se, […] be combined with reusable blocks to implement the [gallery] behavior?

This is good food for thought for the future. I don't necessarily see context-switching container blocks, but I do see some blocks as being able to (and offering to) operate in different contexts.

---

In any case, as a way to move this PR forward, one thing I can do is allow dynamic blocks to fall back to static content on the server:

https://gist.github.com/mcsf/573fd7c42943f89c44ab31efe29d1a81

In short: if a block is server-side registered with a render_callback, this callback will always be called on the server. If the output of that call is empty (cast as '') and the block is not void (i.e., not a self-closing <!-- wp:foo/foo {} /--> but rather <!-- wp:foo/foo {} -->static content<!-- /wp:foo/foo -->), then that static content is used rather than the empty dynamic output. This allows the following for Gallery:

function render_block_core_gallery( $attributes ) {
	return $attributes['useAttachedImages'] ? '[gallery]' : null;
}

---

If the piece described above is something we're OK adding, then we need to agree on an acceptable UX for the MVP of this dual gallery block. Some approaches:

• When converted from classic/paste to blocks, any [gallery] defaults to the static version (i.e. grabs the list of attached images from the server and inserts them as block content).
  • In block settings, an option is made available to "revert" to proper dynamic behavior but only when the conversion was made from a dynamic gallery (i.e. converting from [gallery ids="1,2,3"] wouldn't lead to exposure of this setting).
  • or This option is always available (as an advanced setting?).
• or When converted from classic/paste to blocks, a dynamic gallery shortcode defaults to the dynamic version of the gallery block. In block settings, the option to convert to static is present.
  • Visually, a dynamic GalleryBlock in the editor looks just like a static one, displaying the images attached to the post.
  • or Visually, a dynamic GalleryBlock in the editor makes it clear that it's in a special mode (how?).

[karmatosed]

Looping @melchoyce in here as this is dipping into the Customization focus.

My own feelings are based on @mscf's summary my preference is:

When converted from classic/paste to blocks, any [gallery] defaults to the static version (i.e. grabs the list of attached images from the server and inserts them as block content).
In block settings, an option is made available to "revert" to proper dynamic behavior but only when the conversion was made from a dynamic gallery (i.e. converting from [gallery ids="1,2,3"] wouldn't lead to exposure of this setting).
or This option is always available (as an advanced setting?).

[karmatosed]

I am removing design feedback label as it does have that.

[danielbachhuber]

@mcsf @karmatosed Given the intersection of today's date and the state of this pull request, I have a proposal that lets us kick this decision into the future:

1. For Try Gutenberg, the Gallery Block doesn't support a concept of useAttachedImages. We can open an issue about potentially supporting such a concept in the future, but it wouldn't be a feature the Gallery Block supports in the near-term.
2. For Try Gutenberg, a standalone [gallery] shortcode would convert to a Shortcode Block. A [gallery ids="1,2,3"] shortcode would convert to a Gallery Block.

Ultimately, it doesn't appear we've yet come to a "good" option, only varying degrees of worse options. I don't think this UX is important enough to try to rush a decision on. Plus, we still have aspects like #5972 to deal with.

Also, based on my Swarm, @westonruter is on vacation right now so I expect any input from him would be delayed.

[mcsf]

1. For Try Gutenberg, the Gallery Block doesn't support a concept of `useAttachedImages`. We can open an issue about potentially supporting such a concept in the future, but it wouldn't be a feature the Gallery Block supports in the near-term.

2. For Try Gutenberg, a standalone `[gallery]` shortcode would convert to a Shortcode Block. A `[gallery ids="1,2,3"]` shortcode would convert to a Gallery Block.

What a delay on this one! I think this is a good "less bad" option.

Edit: Moving the following to new issue #11551.

The only glitch is that the transformation API for shortcodes is different from the other ones (block, raw, files) in that it doesn't allow the consumer to provide a function that ultimately decides to call createBlock with whatever block type — essential to distinguish createBlock( 'core/gallery' ) from createBlock( 'core/shortcode' ).

@iseulde, do you remember if there was a motivation to keep the API for shortcodes different? Also, am I right in thinking that we currently can't proceed without tweaking the API?

[mcsf]

Closing. Opening #11551 instead.