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: Add alternate caption style #8344

Closed
wants to merge 1 commit into from

Conversation

noisysocks
Copy link
Member

Description

Fixes #8030.

Adds an option to show gallery captions underneath each image instead of overlaid on top of each image.

How has this been tested?

  1. Insert a gallery block. Ensure there's a good selection of images with varied aspect ratios
  2. Ensure that the gallery looks and works good in the editor and on the frontend
  3. Toggle on Display Captions Below Images
  4. Ensure that the block looks and works good in the editor and on the frontend

Screenshots

screen shot 2018-08-01 at 14 11 05

@noisysocks noisysocks added [Type] Enhancement A suggestion for improvement. [Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress labels Aug 1, 2018
@noisysocks
Copy link
Member Author

noisysocks commented Aug 1, 2018

@kjellr @jasmussen: This is really close! But I need help with some CSS.

Per the design, images that are on the same row should have the same height:

But in my implementation, this doesn't happen:

screen shot 2018-08-01 at 14 22 46

I've put a green outline on .blocks-gallery-item to help illustrate what's happening: each .blocks-gallery-item is expanding to fill the row, the figcaption is placed at the bottom of the .blocks-gallery-item, and then the img only fills the leftover space.

Will play with this tomorrow when I have a fresh mind, but in the mean time, any thoughts on how to make this behave as in the design?

@kjellr
Copy link
Contributor

kjellr commented Aug 1, 2018

Will play with this tomorrow when I have a fresh mind, but in the mean time, any thoughts on how to make this behave as in the design?

🤔 This is not straightforward with the current DOM structure of galleries.

Technically, I think we can make this work for is-cropped galleries by setting an explicit value for the height of the img element. But that's not 100% ideal, and I'm struggling with getting the non-cropped variant to work as designed.

I'm going to take a mental break too, but I started working through this in a codepen here in case anyone wants to poke around.

@kjellr
Copy link
Contributor

kjellr commented Aug 2, 2018

@noisysocks Ok, check out this codepen:

https://codepen.io/kjellr/pen/EpLWZM

If we switch the figure to use CSS Grid, with 2fr 1fr columns, we can get everything to behave almost as expected. Two caveats:

  • Since the units are relative to each other... if someone entered a very long (tall) caption, the image above it would get tall too. Similarly, if there's a really tall image, the space below it would be proportionally tall as well. 😕
  • This also leaves extra space below the images, even if there are no captions in that particular row.. we'd want to collapse that in the deselected state somehow.

So not optimal, but feels closer... paging @jasmussen for ideas, since I know you're a wizard at this stuff. 🙂

Adds an option to show gallery captions underneath each image instead of
overlaid on top of each image.
@jasmussen
Copy link
Contributor

I have some concern about the complexity of adding this option to a core WordPress block. It seems like this could be a separate gallery block, and that the stock gallery block should instead focus on being as minimal as possible, and the overlay captions are a way to do that.

There's also the option to add a CSS class to a gallery and modify the presentation either using just a theme style, or a theme style and an editor style both. I'd also like more options to extend the gallery, but still keep the core offering simple. Remember this is something we'll have to maintain for eternity.

However in absence of that concern, the style itself looks nice, and I think that if we wanted to ship it we'd simply want to make sure that the gallery looks unchanged. This seems to be the case:

screen shot 2018-08-06 at 10 17 18

screen shot 2018-08-06 at 10 17 33

The above, plus Kjell's fixes, and I wouldn't object, even if I still have that concern noted.

Oh, one more thing — we have to support IE11 😢 — be sure to test. The gallery is already rather delicately balanced to work in IE, and even Edge, and some of the flexiness is turned off in order to work.

@kjellr
Copy link
Contributor

kjellr commented Aug 6, 2018

There's also the option to add a CSS class to a gallery and modify the presentation either using just a theme style, or a theme style and an editor style both. I'd also like more options to extend the gallery, but still keep the core offering simple. Remember this is something we'll have to maintain for eternity.

If we're having this much trouble pulling the captions out of the images by just changing CSS, theme developers are also going to have trouble. So it'd be great to solve it one way or another.

I ran into this issue personally while designing a theme — the gradient overlays didn't match styles I was using anywhere else in the theme, but since there was no straightforward way to pull the captions out, I left it as is.

In any case, the current implementation in the branch currently looks fairly disjointed:

screen shot 2018-08-06 at 10 54 30 am

... and gets substantially worse if you disable cropping:

screen shot 2018-08-06 at 10 54 56 am

I think my CSS updates will solve that to some extent, but it'll also introduce more potential bugs. So this still needs some work.

Oh, one more thing — we have to support IE11 😢 — be sure to test. The gallery is already rather delicately balanced to work in IE, and even Edge, and some of the flexiness is turned off in order to work.

A sidenote, but when I started checking out how this works there, but ran into other issues. 😭

@samikeijonen
Copy link
Contributor

I'm not sure what's the status of this ticket but I feel like extra option is not needed.

  • By default gallery styles with captions should look OK. Perhaps show them under the image by default.
  • As a themer option "Display Captions Below Images" would mean that I have to have default captions styles inside the image. And that doesn't feel good to me.

@noisysocks
Copy link
Member Author

Closing due to staleness.

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 [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants