fix(accordion): ensure disableContentPadding works with subtle variant #6728
Conversation
| @@ -111,519 +107,3 @@ export const Grouped = ({ ...args }) => ( | |||
| ); | |||
|
|
|||
| Grouped.storyName = "grouped"; | |||
|
|
|||
There was a problem hiding this comment.
praise(non-blocking): nice bit of tidying up here 😄
| ${({ disableContentPadding }) => | ||
| disableContentPadding && | ||
| css` | ||
| padding: 0; |
There was a problem hiding this comment.
suggestion(non-blocking): I see that we've used the Design Tokens elsewhere in this file to specify values. Might be nice to update this value too whilst we're moving this code block.
| padding: 0; | |
| padding: var(--spacing000); |
There was a problem hiding this comment.
For consistency sake I think it's a fair argument but it is a strange design token when you think about it. When would 0 spacing ever need to be changed from 0? Not entirely sure why that exists, I guess maybe there's a world when you decide 0 spacing should actually be a little spacing...?
| @@ -7,28 +7,6 @@ import { Checkbox } from "../checkbox"; | |||
| import { Dl, Dt, Dd } from "../definition-list"; | |||
| import Link from "../link/link.component"; | |||
|
|
|||
| export const Grouped = () => ( | |||
There was a problem hiding this comment.
question(non-blocking): was this meant to be deleted? We have a Chromatic snapshot capturing this test story that is all.
There was a problem hiding this comment.
I deleted this as it seemed copied over from the test stories and was not being used anywhere, I also don't think we have any snapshots for this component as its set to disableSnapshot: true on the test stories.
| ${({ disableContentPadding }) => | ||
| disableContentPadding && | ||
| css` | ||
| padding: 0; |
There was a problem hiding this comment.
For consistency sake I think it's a fair argument but it is a strange design token when you think about it. When would 0 spacing ever need to be changed from 0? Not entirely sure why that exists, I guess maybe there's a world when you decide 0 spacing should actually be a little spacing...?
Fixes issue where disableContentPadding prop had no effect in subtle variant. fix #6723
|
🎉 This PR is included in version 135.1.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix #6723
Proposed behaviour
disableContentPaddingprop removes padding in "subtle" variant.Current behaviour
disableContentPaddingprop has no effect on "subtle" variant.Checklist
d.tsfile added or updated if requiredQA
Additional context
Testing instructions