Skip to content

Conversation

cdransf
Copy link
Member

@cdransf cdransf commented Oct 8, 2025

Description

Addresses an issue where the content of a card can have a border radius that does not match that of the parent card.

Related issue(s)

  • fixes SWC-1034

Screenshots (if appropriate)

Screenshot 2025-10-13 at 10 51 32 AM

Manual review test cases

  1. Navigate to one of the VRTs for this pull request.
  2. Select CardStories
  3. Select smallQuiet
  4. See that the corners of the image contained within the card are not rounded.
  5. Drag the slider past the component to see the new state.
  6. See that the new corner radius is now consistent with that of the card that contains it.

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be published.
  • I have included updated documentation if my change required it.

@cdransf cdransf self-assigned this Oct 8, 2025
@cdransf cdransf added the WIP label Oct 8, 2025
Copy link

changeset-bot bot commented Oct 8, 2025

⚠️ No Changeset found

Latest commit: c467d85

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Oct 8, 2025

📚 Branch Preview

🔍 Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Deployed to Azure Blob Storage: pr-5796

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

Copy link
Contributor

github-actions bot commented Oct 8, 2025

Tachometer results

Currently, no packages are changed by this PR...

@coveralls
Copy link
Collaborator

coveralls commented Oct 8, 2025

Pull Request Test Coverage Report for Build 18501045955

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.957%

Totals Coverage Status
Change from base Build 18472233594: 0.0%
Covered Lines: 34182
Relevant Lines: 34716

💛 - Coveralls

@Rajdeepc
Copy link
Contributor

Rajdeepc commented Oct 9, 2025

Can you also attach the github here too?

@cdransf cdransf marked this pull request as ready for review October 9, 2025 15:17
@cdransf cdransf requested a review from a team as a code owner October 9, 2025 15:18
@cdransf
Copy link
Member Author

cdransf commented Oct 9, 2025

Can you also attach the github here too?

Ah, it was reported in Jira, I'm not sure there's anything from GitHub to attach.

@Rajdeepc
Copy link
Contributor

Rajdeepc commented Oct 9, 2025

can you please list some instructions for other reviewers to verify this change?

@cdransf
Copy link
Member Author

cdransf commented Oct 9, 2025

can you please list some instructions for other reviewers to verify this change?

Added! ✨

Manual review test cases

  1. Navigate to one of the VRTs for this pull request.
  2. Select CardStories
  3. Select smallQuiet
  4. See that the corners of the image contained within the card are not rounded.
  5. Drag the slider past the component to see the new state.
  6. See that the new corner radius is now consistent with that of the card that contains it.

Copy link
Contributor

@Rajdeepc Rajdeepc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few questions here:

  1. We are adding a deliberate border-radius to the card content - Is this signed off from the design?
  2. Why are we not allowing customisations? What happens to products which doesnt want a border-radius on the content?

Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @Rajdeepc that we're introducing some VRT diffs in a couple other quiet stories that I imagine we want to avoid. When I wrote the ticket, I was hoping we could target just the small-quiet story to get the image border-radius to match the rest of the container like the other cards.

@cdransf cdransf force-pushed the cdransf/card-small-quiet-border-radius branch 2 times, most recently from d671e4c to f2a0f5c Compare October 13, 2025 15:17
@cdransf cdransf force-pushed the cdransf/card-small-quiet-border-radius branch from f2a0f5c to 1e40f17 Compare October 13, 2025 16:41
@cdransf
Copy link
Member Author

cdransf commented Oct 13, 2025

I agree with @Rajdeepc that we're introducing some VRT diffs in a couple other quiet stories that I imagine we want to avoid. When I wrote the ticket, I was hoping we could target just the small-quiet story to get the image border-radius to match the rest of the container like the other cards.

That makes sense! I've updated it to target that variant specifically and the VRTs now reflect a change to only that variant.

@Rajdeepc
Copy link
Contributor

Once you update the hash, then we are good to go!

Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good! I didn't catch the VRTs before the hash was updated (that's my fault for being slooow) but the story looks great now!

Thank you! ❤️

@cdransf cdransf merged commit ad84aaf into main Oct 14, 2025
27 checks passed
@cdransf cdransf deleted the cdransf/card-small-quiet-border-radius branch October 14, 2025 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants