Consolidate MessageBanner srcset logic and document image migration plan - #14352
Draft
vdeksne wants to merge 5 commits into
Draft
Consolidate MessageBanner srcset logic and document image migration plan#14352vdeksne wants to merge 5 commits into
vdeksne wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a small step in the WS-63 image-component spike: it removes duplicated srcset/sizes construction from MessageBanner by reusing the existing getSrcSets utility, and adds a migration-plan document to guide follow-up consolidation work across other image consumers.
Changes:
- Switched
MessageBannerto use#app/utilities/getSrcSetsinstead of locally rebuildingsrcSet/sizes. - Added a new MDX document auditing current duplication and proposing a staged migration order toward a shared image-data API.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/app/components/MessageBanner/index.tsx | Replaces inline srcset/sizes math with the shared getSrcSets utility. |
| docs/Image-Component-Migration-Plan.mdx | Documents current image-prep duplication and an incremental migration plan. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+40
to
+45
| const imageSrcSets = getSrcSets({ | ||
| imageUrlTemplate: image, | ||
| mq, | ||
| imageWidthSmall: styles.IMAGE_WIDTH, | ||
| imageWidthLarge: styles.IMAGE_WIDTH_GROUP_3_MIN_WIDTH, | ||
| }); |
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.
Resolves JIRA: WS-63
Summary
This is a proof-of-concept step from the WS-63 image-component spike.
The audit confirms that
StoryPromoandPromo.Imagealready use the modernImageprimitive. The remaining opportunity is to consolidate the duplicated logic
that prepares image URLs,
srcSet, MIME types, fallback source sets, andresponsive sizes before rendering that primitive.
As a low-risk first step,
MessageBannernow reuses the existinggetSrcSetsutility rather than constructing responsive image attributes locally. This does
not change the rendered image behaviour.
The migration plan below records the audit findings, proposed target
architecture, risks, and incremental follow-up work.
Image Component Consolidation Plan
Current Architecture
Target Architecture
Proposed Delivery Order
Image-Component-Migration-Plan.mdx.getSrcSets.Code changes
srcSetandsizesconstruction inMessageBannerwith
#app/utilities/getSrcSets.Image-Component-Migration-Plan.mdx,documenting the audit, architecture diagrams, risks, and delivery order.
Testing
npx jest src/app/components/MessageBanner/index.test.tsx.MessageBannerimage source,srcSet, andsizesvalues.Useful Links