-
Notifications
You must be signed in to change notification settings - Fork 204
[IRN-6176][BpkCardList] use negative margin to remove bottom padding #3990
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
[IRN-6176][BpkCardList] use negative margin to remove bottom padding #3990
Conversation
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
1 similar comment
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
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.
Pull Request Overview
This PR removes the built-in top and bottom padding from the BpkCardList carousel component to better align with design requirements. The change uses negative margins combined with padding to maintain space for potential box-shadows while eliminating the visual padding that prevented matching design specifications.
- Replaces existing
padding-topandpadding-bottomwith negativemargin-blockandpadding-blockproperties - Adds spacing above accessory elements to maintain layout consistency
- Implements responsive behavior for mobile breakpoints
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| BpkCardListCarousel.module.scss | Replaces padding with negative margin approach to remove visual padding while preserving shadow space |
| BpkCardListRowRailContainer.module.scss | Adds top margin to accessory elements and imports tokens for spacing values |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
packages/bpk-component-card-list/src/BpkCardListRowRail/BpkCardListCarousel.module.scss
Show resolved
Hide resolved
| margin-block: -(tokens.bpk-spacing-base()); | ||
| padding-block: tokens.bpk-spacing-base(); |
Copilot
AI
Oct 6, 2025
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.
[nitpick] The mobile breakpoint uses a different spacing value (bpk-spacing-base vs bpk-spacing-lg) but applies the same negative margin technique. This duplication could be reduced by using CSS custom properties or a mixin to avoid repeating the pattern.
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
1 similar comment
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
2 similar comments
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
kerrie-wu
left a comment
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.
LGTM
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
kerrie-wu
left a comment
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.
Approve and wait for major release merge
|
Visit https://backpack.github.io/storybook-prs/3990 to see this build running in a browser. |
The designs for the expandable pricing option use BpkCardList. But this component has top and bottom padding on the carousel component within it (see screenshot), making it impossible to match the designs.
After speaking with Backpack it was agreed that it doesn't make sense for the Carousel component to come with built-in top and bottom padding
To remove this padding while also ensuring that any potential box-shadows on card components within the carousel aren't clipped, this PR introduces a consistent top and bottom padding and negative margin value.
What has changed
0.25 rem of spacing has been removed from the top of the BpkCardListCarousel. This means that, in combination with this PR, it will be possible for consumers to use the row and rail layouts without any top padding (when no title, chip group or cta button props are provided). The spacing change has been achieved using a consistent positive padding and negative margin value (1rem) to prevent any box shadows being cut off
1 rem of spacing has been removed from the bottom of BpkCardListCarousel, while 1 rem of spacing has been added to the top of the accessory. This will mean that there is NO visual change when the accessory is rendered. However, when the accessory isn't rendered there will no longer be any bottom padding baked into the component. Once again the spacing change has been achieved using a consistent positive padding and negative margin value (1rem) to prevent any box shadows being cut off
With accessory
Without accessory
Migration for breaking change
RAILorROWlayout without an accessory being rendered, consider adding 1rem of spacing below the Card List component to restore the 1rem of spacing that has been removedRemember to include the following changes:
[Clover-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md