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

Add Block: Gallery #317

Closed
mtias opened this issue Mar 23, 2017 · 19 comments
Closed

Add Block: Gallery #317

mtias opened this issue Mar 23, 2017 · 19 comments
Assignees
Labels
[Feature] Blocks Overall functionality of blocks [Type] Task Issues or PRs that have been broken down into an individual action to take
Milestone

Comments

@mtias
Copy link
Member

mtias commented Mar 23, 2017

Attributes

  • Alignment: None, Left, Right, Center, Full Bleed.
  • Ordering of images.
  • Image captions and links.

Markup (to be determined)

<!-- wp:gallery -->
<!-- /wp -->

States (see all)

image

@mtias mtias added Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Task Issues or PRs that have been broken down into an individual action to take labels Mar 23, 2017
@mtias
Copy link
Member Author

mtias commented Mar 29, 2017

Question: should types of gallery become separate blocks or attributes of this core block (Jetpack tiled galleries, for instance).

@jasmussen
Copy link
Contributor

Question: should types of gallery become separate blocks or attributes of this core block (Jetpack tiled galleries, for instance).

Depends on what's possible.

Here's an interaction I think would be an amazing experience, we can then potentially let that guide what's possible: drag 5 images from the desktop into the editor and it becomes a default gallery. Click this gallery block, and pick a different style of gallery from a list or toggle control — for example you might want a masonry gallery instead of a tiled list of thumbnails.

Whether the masonry gallery becomes a different block type you can choose to insert or switch to using the Switcher, or whether the basic gallery block simply provides two gallery styles on its own, either could work.

How much of this would be doable for us?

@kopepasah
Copy link
Member

Question: should types of gallery become separate blocks or attributes of this core block (Jetpack tiled galleries, for instance).

I feel like including all types of galleries in one block (as attributes) allows for the most simplicity for the user.

Most of what is possible depends on our implementation. The current gallery only relies on the ID of the media, which give quite a bit of flexibility.

While the mockups demonstrate a few states, my biggest concern is how this interacts with the current method of inserting using the media manager (if at all). I do like the idea of giving users the capability for reorganizing images without the use of the media manager, but how would reorganization occur with Jetpack's tiled layout type (which required Photon and currently must be arranged in the media manager)? This does not even account for the other layout types (present now and in the future).

  • Would it seem odd to allow organization on in the editor for one type and not the other?
  • Would it make more sense to leave all the photos in the editor similar to the media manager and have the preview display the layout of the selected type?
  • Would it make more sense to have reorganization of galleries to happen within the media manager?

Seems to me that a middle ground between making changes within the editor and the media manager is the best most viable option.

@jasmussen
Copy link
Contributor

You make some solid points, @kopepasah.

Perhaps it's good to look at the mockups as a vision for, and then look at the implementation of that vision in steps. Perhaps in the first version of this block, it's okay to not have a block UI for captioning and linking, and offload that onto the media manager (which could be invoked by a button in the toolbar). Then as time permits, we can augment the gallery with a block UI.

I feel like including all types of galleries in one block (as attributes) allows for the most simplicity for the user.

I agree and disagree. I agree it would be good to have a solid core gallery block that has the attributes mentioned on this ticket, and encourage plugins to augment that block.

But we also want to make the API solid enough that anyone can implement their own gallery block that perhaps has an entirely different set of attributes that don't make sense for the core block to have. It could be a slideshow with custom auto advance attributes, or even a snapchat-esque story gallery with SVG stickers and filters overlaid. Okay admittedly that last one is a stretch.

@joyously
Copy link

I think you are missing one thing: what is the intent of the gallery block?
If it is to replace the shortcode, it should behave that same way: store only parameters in the content that dynamically generate the gallery on page load.
If the intent is to provide a content block that happens to be called gallery, it should store the markup that is the images in the correct order and layout as chosen by the user. (static HTML)
I don't think the "gallery block" should do both. It should be one or the other. There are Media Workflows for both already. That shouldn't have to happen in the editor. If a user inserts an image, the editor can move that around. If they insert five, they can move each one around. It isn't any different, if it's just content in the post.

@jasmussen jasmussen added this to the Alpha milestone Apr 20, 2017
@kopepasah
Copy link
Member

I agree with you completely @joyously. Since developer will have the ability to add custom blocks in the future, this 'block called gallery' should not be thought of as a replacement for the [gallery] shortcode.

If we can all concur on this method, I would like to start on this block.

@aduth
Copy link
Member

aduth commented Apr 25, 2017

this 'block called gallery' should not be thought of as a replacement for the [gallery] shortcode.

There's been some prior discussion on supporting legacy content, most recently with some API explorations at #413 (now out-of-date). The main problem becomes... if there's a shortcode in the content, how do we know how to display it? What controls exist? The idea with a compatibility layer is to allow these content formats to inherit the behavior of another block; in this case, a gallery shortcode behaving the same as the new gallery block. At least as far as showing the block and its interactions, this doesn't have an immediate impact on the original content. If the user were to then manipulate it, well... I think it's still unsettled exactly how we want to save the markup of a "new" gallery (per original comment). But while a block could potentially have multiple input forms (identified as a gallery block, or as a gallery shortcode), the behaviors and output should be consistent.

(Note that I'm not addressing image insertion behavior, or whether there should be multiple blocks for different styles of galleries)

@joyously
Copy link

At least as far as showing the block and its interactions, this doesn't have an immediate impact on the original content.

Well, yes it does. If the "gallery block" is just a wrapper for image blocks, it's quite different than if its attribute is an array of image information. And if it's a shortcode, it's even more different. Especially the plain vanilla shortcode [gallery] which shows all the attached images, because the user can attach more images during the edit and it will need to update, whereas static image blocks don't do that.

If the user were to then manipulate it, well... I think it's still unsettled exactly how we want to save the markup of a "new" gallery (per original comment). But while a block could potentially have multiple input forms (identified as a gallery block, or as a gallery shortcode), the behaviors and output should be consistent.

I disagree. User manipulation of the content would affect each type differently. If it's a wrapper block for images, there is probably no change in content. If the "gallery block" keeps a list of images, then that would have to reflect the changes made. But if it's a shortcode, there is already an interface for manipulating that and there is nothing for the "block" to do but call the shortcode to render it.
I think calling a block a "gallery" is going to cause more confusion than it is worth. But you should clearly define what it is and isn't before you start working on it.

@jasmussen
Copy link
Contributor

This is a crucial discussion to be having, so good that it has many eyes. From the kickoff post:

The editor will endeavour to create a new page and post building experience that makes writing rich posts effortless, and has “blocks” to make it easy what today might take shortcodes, custom HTML, or “mystery meat” embed discovery.

Emphasis mine.

While shortcodes are always going to work through plugins or backwards compatibility, the core of this project is to introduce something better than shortcodes: blocks. The gallery is in a way the proto block, the only existing real block. And so from a philosophical point of view, it seems pretty clear that WordPress should offer a gallery block right out of the box.

Whether that means upgrading the existing gallery block, adding a new one and deprecating and discouraging the use of the old one (not breaking it mind you), I will defer to the tech leads and what works best in implementation.

If it does mean replacing the old one, do remember that the mockups presented in this ticket are simply mockups, and if we find it important to rewind them a bit and bring them closer to the current look and feel of the gallery block, that's an option on the table.

From a creation flow perspective, I suspect it will likely be mostly identical to how galleries are created now. So I thought it might be helpful to show that in mockup form.

Insert the gallery block first, you get a placeholder:

gallery-placeholder

Click the media library button to invoke it:

media-library

Simply dragging 9 images from the desktop onto the editor should ideally have the same effect, and skip the placeholder step.

@kopepasah
Copy link
Member

if there's a shortcode in the content, how do we know how to display it? - @aduth

In my opinion, the rendering of shortcode should remain unchanged to provide backwards compatibility. All rendering should happen as currently happens in TinyMCE and we should not require developers (at least in the beginning) to rewrite all of their shortcodes.

@jasmussen I like the flow proposed of inserting a gallery. Since "blocks" are meant to " introduce something better than shortcodes", I would argue that creating an entirely new way of creating galleries is the best method. While allowing the [gallery] shortcode to handle backwards compatibility.

In short: create a gallery block which is in no relation to the [gallery] shortcode.

I think calling a block a "gallery" is going to cause more confusion than it is worth - @joyously

I completely agree and maybe calling is something different is ideal.

@joyously
Copy link

So this block is still undefined. Is it dynamic or static content? It seems like the arguments for static are louder than dynamic.

Simply dragging 9 images from the desktop onto the editor should ideally have the same effect, and skip the placeholder step.

I disagree that images dropped onto the editor should make one of these blocks. I don't want to have to fight the editor to be able to show a bunch of images. I see this block as being thumbnails(size choosable) that link to the full image or attachment page, whereas images are just displayed right where they are at the size and alignment chosen for each.

@jasmussen
Copy link
Contributor

Upon discussing how to create this, here are new step by step mockups that omits the placeholder block for galleries.

Place your cursor where you want the gallery:

inserting

Click the inserter, on the side or in the toolbar:

popover

Pick the gallery to open the media library:

gallery

Pick your images and insert:

gallery done

@joyously
Copy link

So this block is still undefined. Is it dynamic or static content?

Still wondering.

@kopepasah
Copy link
Member

We should just try the static implementation and see if it fits into the flow. Having something to test might help use define the requirements more easily.

I am good to start working on this, if so.

@jasmussen
Copy link
Contributor

Hey @kopepasah, I may have promised @mkaz that he'd get to take a first stab at this block soon. But we'd want to keep the initial PR for the gallery block as small as possible, and PR it as early as possible so we can give feedback during this development.

Then features like sorting images, applying block level controls, captions, could happen in separate small PRs. What do you think?

@kopepasah
Copy link
Member

@jasmussen sounds perfectly fine to me, as there is plenty to work on. I agree that this issue should be broken into smaller chunks, once we have a base with which to work.

@westonruter
Copy link
Member

Also take note of parallel work on a core Gallery widget: xwp/wp-core-media-widgets#62

@mtias mtias added [Feature] Blocks Overall functionality of blocks and removed Framework Issues related to broader framework topics, especially as it relates to javascript labels May 3, 2017
@mkaz mkaz mentioned this issue May 9, 2017
@westonruter
Copy link
Member

I noticed that #740 did not implement the insertion of the underlying attachment ID to the gallery block data. I know that PR was just an initial implementation. Has the presence of the IDs been discussed? It is hinted at but not explicitly mentioned in #390. Having the ID for each image is key, for example, to be able to manipulate the image sources in the gallery (e.g. via Photon) and also to obtain the current alt text and captions for the images, if they aren't defined inline with the gallery instance's images.

@mkaz
Copy link
Member

mkaz commented Jun 12, 2017

@westonruter could you create a new issue and explain further with example and I'll implement it. Feel free to tag me so I see it

@mkaz mkaz closed this as completed Jun 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

7 participants