[DCUBESDQLR-2356] Add custom content slot to FullscreenImageCarousel#1098
Merged
[DCUBESDQLR-2356] Add custom content slot to FullscreenImageCarousel#1098
Conversation
…arousel - Add discriminated union FullscreenCarouselItemProps (ImageCarouselItemProps | CustomCarouselItemProps) - Add renderContent per-item render prop for custom slide content - Add itemLabel per-item prop for accessible aria-label overrides - Hide magnifier button for custom content slides - Fall back to SlidePlaceholderImage in thumbnail strip when no src available - Update carousel-level aria-labels to generic wording when any item uses itemLabel
…slot - Add WithCustomContent story with PDF iframe and pdf-icon thumbnail - Update props-table with renderContent and itemLabel docs - Add tests for renderContent, itemLabel aria-labels, magnifier hide, thumbnail placeholder - Remove fileName/fileSize from this branch (belongs to file-info-bar branch) - Add pdf-icon.svg asset from sgw-dl
- Custom items only need thumbnailSrc for the thumbnail strip
There was a problem hiding this comment.
Pull request overview
Adds support for per-slide custom rendering in FullscreenImageCarousel, enabling non-image slides (e.g. iframes/PDF viewers) while updating accessibility labeling, thumbnails, docs, and tests to support mixed item types.
Changes:
- Introduces a discriminated union for carousel items (
ImageCarouselItemProps | CustomCarouselItemProps) with a newrenderContentrender-prop anditemLabel. - Updates carousel rendering to support custom slides, hide zoom controls for them, and show thumbnail placeholders when needed.
- Adds Storybook docs/story coverage plus unit tests for custom content and updated aria-label behaviors.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/fullscreen-image-carousel/fullscreen-image-carousel.tsx |
Implements custom-slide rendering, generic aria-label mode, magnifier hiding, and thumbnail placeholder behavior. |
src/fullscreen-image-carousel/types.ts |
Updates the public item props type to a union and adds renderContent/itemLabel. |
tests/fullscreen-image-carousel/fullscreen-image-carousel.spec.tsx |
Adds tests for custom content rendering, magnifier visibility, thumbnail placeholder, and aria-label wording. |
stories/fullscreen-image-carousel/fullscreen-image-carousel.stories.tsx |
Adds a “WithCustomContent” example using iframe/PDF content and a custom thumbnail. |
stories/fullscreen-image-carousel/fullscreen-image-carousel.mdx |
Documents the new custom content capability and itemLabel impact on aria-labels. |
stories/fullscreen-image-carousel/props-table.tsx |
Documents new item props and updated union semantics in the props table. |
public/img/pdf-icon.svg |
Adds a PDF thumbnail icon used by the Storybook example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- fix leading punctuation in aria-labels when alt/itemLabel is empty - lazy-render custom content only for active and adjacent slides - trim empty itemLabel in delete button aria-label - rename hasCustomItemLabel to hasAnyItemLabel - keep FullscreenCarouselItemProps as base interface for backward compat
mparramont
force-pushed
the
fullscreen-carousel-custom-content
branch
from
193cfa8 to
d90b3cb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot stopped work on behalf of
mparramont due to an error
April 15, 2026 09:43
qroll
requested changes
Apr 15, 2026
- rename types: CarouselItemProps -> FullscreenImageCarouselItemProps, CustomCarouselItemProps -> FullscreenImageCarouselCustomItemProps - deprecate FullscreenCarouselItemProps in favour of FullscreenCarouselImageItemProps - add type discriminants (type?: "image" | type: "custom") - rename isAdjacent to isActiveOrAdjacent - update story: remove first image item, fix thumbnail URL - update mdx docs wording for itemLabel - fix test name, add src fallback thumbnail test - update props-table with new type names and type field
Contributor
Author
|
@qroll all comments addressed, can you re-review please? 🙇 |
qroll
previously approved these changes
Apr 16, 2026
qroll
approved these changes
Apr 16, 2026
Contributor
qroll
left a comment
qroll
left a comment
There was a problem hiding this comment.
updated some documentation and renamed FullscreenImageCarouselImageItemProps (my bad, typo in the original comment)
Contributor
|
the svg file is currently MIA, I'll check in with the dev. it should have been uploaded to our static assets folder as part of this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(Draft pending desk check)
Type of changes
Description of changes
renderContentper-item prop: when provided, the consumer owns the entire slide content (e.g. iframe, PDF viewer, embed)itemLabelper-item prop for accessible aria-label overrides (e.g."PDF"→ "Delete PDF")itemLabel, carousel-level aria-labels switch to generic wording ("Carousel", "Previous item", etc.)src/thumbnailSrcChecklist
Screenshots
Video showing a custom item (PDF) rendering a preview of the PDF, and another custom item with no
thumbnailSrc, which shows the placeholder:Screen.Recording.2026-04-15.at.3.55.55.PM.mov