Polaris-react: [Proposal] Remove overflow:hidden on the Card component

[dfmcphee]

Issue summary

Currently we apply overflow: hidden to the Card component. This raises problems when you need to use something like position: sticky within the Card, because it can't be used on elements with overflow.

We apply this to make sure children get the correct border radius to match the card itself. The most common example is for subdued sections at the end of a card.

I propose we remove the overflow hidden and add these rules to Card.Sections:

border-bottom-left-radius: border-radius();
border-bottom-right-radius: border-radius();

This will solve the majority of use cases and then if any other component needs to stretch the full width/height of a card, they apply the border radius.

The current blanket statement to remove overflow feels heavy handed for those few unique use cases.

Thoughts? Opinions?

[dpersing]

NUKE IT! 🔥

[beefchimi]

FYI We do exactly this in our "Card" component:
https://github.com/Shopify/online-store-ui/blob/master/src/components/SidebarCard/SidebarCard.scss#L21-L33

Happy to send you a link to see it on our Chroma instance if you'd like.

[AndrewMusgrave]

I was actually mentioning the problem with overflow: hidden & position: sticky to Dan this morning when we were running into issues with Card and the new focus-ring. 100% for removing this.

Also as a note, we can't use position: sticky in the resource list header exactly because of overflow hidden in cards 😅

[perrupa]

Samesies

I ran into an issue with this regarding non-polaris tooltips inside a Card.

My problem

I'm trying to implement a rich-text editor that uses popups and the Card's overflow is constraining the tooltip's contents.

Solutioning

It would be very handy to override this prop, or maybe only have it on cards that are sectioned? Assuming that's what it's _needed_ for.

Next Steps

What's blocking us here, lack of a PR to push over the line?

Would @beefchimi 's proposed solution be enough to get rid of the overflow: hidden entirely? I feel like it might, unless you were relying on Card having overflow: hidden for other behaviours (we shouldn't support those cases).

The worry-wort in me thinks this might necessitate a breaking change, but counter-point is the API hasn't changed just possible side-effects....

Thoughts?

cc// @dfmcphee @AndrewMusgrave @dpersing 🎺

[dfmcphee]

I just put up a PR for this at #2806

[BPScott]

Gah, I was going to comment on this a while back but then forgot, sorry.

But basically, yep sounds good to me. I'm wary of the impact of how things poking over the edge of the corner but it sounds like you're only ever supposed to put Card.Sections as a Card's final child anyway.

[alex-page]

This has shipped, closing ✨