Add image number above caption in Hosted Gallery#16207
Conversation
deedeeh
added
maintenance
deedeeh
changed the title
| const emptyCaption = captionHtml === undefined || captionHtml.trim() === ''; | ||
| const hideCredit = | ||
| displayCredit === false || credit === undefined || credit === ''; | ||
| const shouldIncludeShareButton = |
There was a problem hiding this comment.
Removed this to avoid repetition as we can use the same isHostedGallery with just the ! to exclude Share from Hosted Gallery.
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
deedeeh
force-pushed
the
dina/add-picture-index-hosted-gallery
branch
from
d3723a3 to
099df65
Compare
dskamiotis
left a comment
dskamiotis
left a comment
There was a problem hiding this comment.
This looks good to me Dina, nice move with the isHostedContent to be the gate the share button inclusion
| const emptyCaption = captionHtml === undefined || captionHtml.trim() === ''; | ||
| const hideCredit = | ||
| displayCredit === false || credit === undefined || credit === ''; | ||
| const shouldIncludeShareButton = |
| padding: ${space[2]}px 0 ${space[1]}px; | ||
| `} | ||
| > | ||
| {position - 1}/{imagesLength} |
There was a problem hiding this comment.
Just to clarify my understanding why this uses position - 1? The surrounding lightbox code makes position look 1-based:
dotcom-rendering/dotcom-rendering/src/components/LightboxJavascript.tsx
Lines 151 to 155 in 65f695b
although local testing suggests the displayed counter is correct, so I may be missing where this value is normalised earlier?
There was a problem hiding this comment.
I didn't look into the Lightbox file but I looked how position is used in the GalleryCaption component where you can find in normal Gallery the share button got a hash property where the position is used and if you check the share button in the first image in the body of a normal Gallery it will have img-2 that's why I did position - 1.
There was a problem hiding this comment.
That makes perfect sense now, thanks.
Happy to approve this as is nice work! 👍
Optional follow-up when convenient: maybe add a short inline comment noting that body image position is offset by main media (so the first body image is 2), which is why we render position - 1? It may help avoid future confusion?
2 participants
What does this change?
This PR adds the image number above the caption in Hosted Content Gallery.
Why?
Migrating Hosted Content to DCAR and Hosted Gallery is the last page to migrate. The new designs require this change.
Screenshots