WordPress.org

Making WordPress.org

Opened 10 days ago

Last modified 6 days ago

#4888 accepted defect

Caption overflow in screenshots of plugin single page

Reported by: YordanSoares Owned by: dufresnesteven
Milestone: Priority: normal
Component: Plugin Directory Keywords:
Cc:

Description

I'm having an design issue with a screenshot of a plugin WP Two Factor Authentication with Telegram. I just update the screenshots and captions and saw that the captions stay behind the thumbnails when overflow the 600px limit given to the CSS class .plugin-screenshots .image-gallery-slides, wouldn't it be better if the text pushes down to avoid this defect? Here's the defect:

https://i.imgur.com/KVgFtYW.png

I changed the value to 650px just for testing purposses and works great:

https://i.imgur.com/pz33DxY.png

Attachments (1)

4888.diff (728 bytes) - added by dufresnesteven 6 days ago.
Fix overflow hidden on screenshot captions when image height is large and caption is more is 2+ lines of copy.

Download all attachments as: .zip

Change History (3)

This ticket was mentioned in Slack in #meta by tellyworth. View the logs.


10 days ago

@dufresnesteven
6 days ago

Fix overflow hidden on screenshot captions when image height is large and caption is more is 2+ lines of copy.

#2 @dufresnesteven
6 days ago

  • Owner set to dufresnesteven
  • Status changed from new to accepted

In 4888.diff:

This diff sets the slide parent to display: table-cell and its parent to display: table . (Browser Support Chart)

Why was it happening?
We have hardcoded values for the slide max-height: 600px and the image max-height: 550px. This was done to make sure large images didn't extend too far down the page and make the slider hard to use. If the captions would be larger than 50px, it would hide under the bottom bounds of the slide.

Considerations

  • I considered a JS solution where we could get references to the image and the caption to obtain the height and adjust the max-height property for the image but the JS (in part because its React) makes it a bit messy to gracefully store and read the DOM references.
  • We are also looking to replace the slider completely, so I didn't feel it necessary to invest too much time. See #2273.

Browser Test Matrix using BrowserStack

Environment Detail Result Notes
Win 10/8.1 FF 68
Win 10/8.1 Opera 55
Win 10/8.1 Edge 15
Win 10/8.1 IE 11 Not a regression. Already exists.
Win 8 IE 9/10 Not a regression. Already exists.
MacOS Catalina Safari 13
MacOS Sierra Safari 10
MacOS El Capitan Safari 9
iPhone 6 Safari
Android Galaxy 9 Chrome
Android Galaxy 6 UC Browser
Last edited 6 days ago by dufresnesteven (previous) (diff)
Note: See TracTickets for help on using tickets.