USWDS - Button group: Fix margins on nested button groups#5885
Conversation
mahoneycm
left a comment
There was a problem hiding this comment.
Lgtm! Two non blocking comments but this works for me 👍
Testing checklist
- Confirm that elements in nested button groups are the same height as each other
- Confirm that elements in nested button groups are the same height as their parent buttons
- Confirm the code meets standards
mejiaj
left a comment
There was a problem hiding this comment.
Overall looks good. Two comments.
I tested mobile, tablet, and desktop sizes.
| height: 100%; | ||
|
|
||
| .usa-button-group__item { | ||
| display: flex; |
There was a problem hiding this comment.
Is this property required? Not seeing any effect on test when disabled.
There was a problem hiding this comment.
Good catch. It looks like this isn't needed anymore. I removed this in 8a926e3
There was a problem hiding this comment.
The test prefix is added at the start of the filename. For example: test-usa-button-group--nested.twig.
I realize component template is hardcoded, but in the future we should avoid additional hardcoded components to allow for easier test template creation. As-is this hardcoded test template is okay.
There was a problem hiding this comment.
I updated the file names in the button group /test-patterns/ directory to reflect the common naming convention for these files in 85f2945
mejiaj
left a comment
There was a problem hiding this comment.
Looks good. I tested in Chromium 125, Firefox 127, and Safari 17.4.1.
Summary
Improved styles for nested button groups. Now, nested button groups should match the height of their parents.
Breaking change
This is not a breaking change.
Related issue
Closes #5825
Related pull requests
Changelog PR
Preview link
Nested button group test story
Problem statement
Buttons in nested button groups have mismatched heights.
Solution
This PR adds styles for nested button groups so that the nested buttons are the same height as their parents.
Testing and review