fix: Container with empty content has incorrect header border radius #663
Conversation
src/container/index.tsx
Outdated
| disableContentPaddings={disableContentPaddings} | ||
| {...externalProps} | ||
| {...baseComponentProps} | ||
| __hiddenContent={disableContentPaddings && !props.children} |
There was a problem hiding this comment.
is disableContentPaddings necessary?
this should work regardless of padding, customers used this as a hack but I think it should work without it
There was a problem hiding this comment.
Makes sense! Will update it.
| </Container> | ||
| </div> | ||
| <Container | ||
| disableContentPaddings={true} |
There was a problem hiding this comment.
I would rather leave it here because without disableContentPaddings the border looks correct. It is to make sure the fix still works even with disableContentPaddings.
Codecov ReportBase: 92.97% // Head: 92.98% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #663 +/- ##
=======================================
Coverage 92.97% 92.98%
=======================================
Files 574 574
Lines 16766 16781 +15
Branches 4628 4631 +3
=======================================
+ Hits 15588 15603 +15
Misses 1101 1101
Partials 77 77
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| disableContentPaddings={disableContentPaddings} | ||
| {...externalProps} | ||
| {...baseComponentProps} | ||
| __hiddenContent={!props.children} |
There was a problem hiding this comment.
Does this mean that all internal use of container remain unfixed? Why is that needed?
This fix should be applied to internal instances too
There was a problem hiding this comment.
Basically, this line should become
[styles['with-hidden-content']]: !children || __hiddenContent
There was a problem hiding this comment.
Hi Boris, thanks for checking it. I was not sure we should not use container without content internally at all, It should be either ExpandableSection or Header? But I agree it makes sense the fix should be for all cases.
3 participants
Description
When container has no content and disableContentPaddings, the bottom border radius are overlapped by header. The change adds border radius when no content in the container, as we did for collapsed ExpandableSection.
For test, add one more instance in simple.page, it will show in the screenshot tests.
Related links, issue AWSUI-20171
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.