-
Notifications
You must be signed in to change notification settings - Fork 40
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
Fix broken placeholder images #923
Conversation
|
🟢 No design token changes found |
🟢 No visual differences foundOur visual comparison tests did not find any differences in the UI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR fixes broken placeholder images by replacing remote placeholder URLs with a local static asset and updating associated alt text to reflect the new background color.
- Replaced remote placeholder image URLs with "/brand/assets/placeholder.png" in several components, tests, and stories.
- Updated alt text descriptions from "off-white" to "gray" to match the local image’s background color.
Changes
File | Description |
---|---|
packages/e2e/integration-tests/fixtures/KitchenSink.tsx | Updated placeholder image source and alt text in the KitchenSink fixture. |
packages/react/src/Image/Image.test.tsx | Replaced remote image source with local "/brand/assets/placeholder.png" in tests. |
packages/react/src/Card/Card.test.tsx | Updated Card image test with the local placeholder image. |
packages/react/src/Footnotes/Footnotes.examples.stories.tsx | Revised alt text description in the Footnotes stories. |
packages/react/src/Image/Image.features.stories.tsx | Changed the imported placeholder image and updated alt text in Image feature stories. |
packages/react/src/Image/Image.stories.tsx | Updated image source and alt text in Image stories. |
packages/react/src/Bento/BentoItem.features.stories.tsx | Updated the placeholder image import in BentoItem feature stories. |
packages/react/src/Hero/Hero.features.stories.tsx | Replaced the placeholder image in Hero stories. |
packages/react/src/Card/Card.features.stories.tsx | Updated the placeholder image import in Card feature stories. |
Copilot reviewed 136 out of 136 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
packages/react/src/Card/Card.features.stories.tsx:3
- Consider revising the alt text to use 'a gray' instead of 'an gray' for grammatical correctness.
<Card.Image src={placeholderImage} alt="placeholder, blank area with an gray background color" />
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! thanks Josh
Summary
Fixes broken placeholder images by replacing remote placeholders with local.
List of notable changes:
placeholder-600x400.png
withplaceholder.png
which is a 600px × 400px image with background#6B6B6B
placeholder.com
to use this new localplaceholder.png
Where we previously used many different sizes of placeholder we now only use this single 600px × 400px image. The only place where this is not quite ideal is in the
Image
component where we referencesrcset
and the<source>
element. I've opted to keep those pointing at the single placeholder file, rather than creating multiple placeholders of differing resolutions.What should reviewers focus on?
Supporting resources (related issues, external links, etc):
Contributor checklist:
update snapshots
label to the PR)Reviewer checklist: