Tiled gallery block: allow adding images from media gallery after adding images to block

Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.

https://jetpack.com/

Other

1.58k stars 797 forks source link

Tiled gallery block: allow adding images from media gallery after adding images to block #11877

Open simison opened 5 years ago

simison commented 5 years ago

When adding Tiled gallery block, user can choose to upload or add image via media gallery:

After adding images, user can only choose to upload images:

We should keep "Media library" option here, too.

We should follow closely what core gallery does for this section and possibly aim to componentize it: https://github.com/WordPress/gutenberg/pull/9682 and https://github.com/WordPress/gutenberg/pull/12367

Relates to permissions checking: https://github.com/WordPress/gutenberg/blob/bcf55ff095fb9cbfee37aef24c422fbbf9661562/docs/data/data-core.md#hasuploadpermissions

sirreal commented 5 years ago

I was implementing this in #30863 where I attempted to unify the empty gallery placeholder and the add media placeholder, but have hit significant roadblocks.

simison commented 5 years ago

I think we should piggyback on this implementation once it merges https://github.com/WordPress/gutenberg/pull/12367

sirreal commented 5 years ago

I think we should piggyback on this implementation once it merges WordPress/gutenberg#12367

Yep, that implementation is essentially what I propose in #30863. A large part of MediaPlaceholder is duplicated. I suspect https://github.com/WordPress/gutenberg/pull/9682 to experience the same issues I discovered in #30863.

simison commented 5 years ago

Do you think it's related to media gallery bugs fixed in https://github.com/WordPress/gutenberg/pull/12435 ?

sirreal commented 5 years ago

No, there are just limitations to the MediaPlaceholder. See the full description: https://github.com/Automattic/wp-calypso/pull/30863#issue-253992640

Specifically, there's no clear way around this limitation:

When uploading media via the placeholder button to an existing gallery, it completely replaces the existing images.

When uploading media via drag and drop to an existing gallery, it completely replaces the existing images.

Otherwise, the MediaPlaceholder implementation is quite nice.

simison commented 5 years ago

cc @jorgefilipecosta

jorgefilipecosta commented 5 years ago

Hi @simison, @sirreal, My plan was to after merging https://github.com/WordPress/gutenberg/pull/12367 extract a component that allows rendering the gallery appender. After reading this PR and the related one https://github.com/Automattic/wp-calypso/pull/30863, I noticed that the gallery appender being implemented in https://github.com/WordPress/gutenberg/pull/12367 is just equivalent to MediaPlaceholder without label and icon, with the media library button opening the media library in a different state, and with some custom styles. It may be possible refactor https://github.com/WordPress/gutenberg/pull/12367 to use MediaPlaceholder in the gallery appender by adding two additional optional props one that enables a custom style and one that allows opening the media library in a specific state (the add to gallery mode). Feel free to share your thoughts about this idea.

sirreal commented 5 years ago

Thanks @jorgefilipecosta! What you outline is aligned with my ideas 👍

Two things I'd add:

MediaPlaceholder needs to take into account that uploaded images, via dialog or drag and drop, should be appended to the current gallery rather than replace it as they do now.

MediaPlaceholder includes DropZone. Core Gallery and Tiled Gallery remove the appender section when the block is not selected. If we rely on MediaPlaceholder and its DropZone, we cannot remove it when the block is not selected or we lose the DropZone. It would be nice to be able to remove the appender UI but keep the DropZone functionality when the block is not selected. In #30863 I've done this with CSS.

sirreal commented 4 years ago

There was some work in https://github.com/Automattic/wp-calypso/pull/30863 that could be brought over for this if anyone is interested in picking it up.

annezazu commented 3 weeks ago

Just ran into this on my site (simple site on WPCOM using 6.6.1):

https://github.com/user-attachments/assets/9e955030-8c82-4365-a3bf-e44193f3b95b

This is rough especially when you're trying to quickly add images in. Something also seems broken with the upload an image button. It would be great to repick this work up.