[LegacyCard] overflow clip to protect border radius

[mattkubej]

WHY are these changes introduced?

Fixes #9252

It is possible for child elements to sit on top and overflow the corners of the LegacyCard, which hid the border radius. This specifically addressed the use case of an IndexTable within a LegacyCard, but as seen within Chromatic there were other areas that had overflow issues such as SkeletonTabs.

Note: It does look like the Chromatic build highlights changes that do not appear to be related to the changes within this PR.

As @nickpresta highlighted below, using hidden will cause issue with position: sticky, so this PR utilizes clip. This does mean that any sticky elements that are children of this card would also be clipped, but will continue to be sticky.

WHAT is this pull request doing?

Prevents child elements of LegacyCard from overflowing onto the corners of the container. This insures the border radius stays in tact.

https://www.w3.org/TR/css-backgrounds-3/#corner-clipping

How to 🎩

• Storybook
• Spin
• Chromatic

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated the component's README.md with documentation changes
• Tophatted documentation changes in the style guide

[github-actions]

size-limit report 📦

Path	Size
polaris-react-cjs	243.03 KB (0%)
polaris-react-esm	157.33 KB (0%)
polaris-react-esnext	219.82 KB (+0.01% 🔺)
polaris-react-css	47.45 KB (+0.01% 🔺)

[mattkubej]

/snapit

[github-actions]

🫰✨ Thanks @mattkubej! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris-cli@0.0.0-snapshot-release-20230524183650

yarn add @shopify/polaris@0.0.0-snapshot-release-20230524183650

[nickpresta]

Adding overflow: hidden to the <LegacyCard /> will break position: sticky on children within the card (e.g. look at this example)

Looking at your Code Sandbox example, could we instead get away with using overflow: clip? It seems to have the same visual effect and doesn't break position: sticky, with the only real tradeoff being that clip prevents programmatic scrolling whereas hidden allows it. This seems like a fine tradeoff, to me, since the use of position: sticky seems more important to maintain than the use of programmatic scrolling within a <LegacyCard />.

Browser support seems fine for overflow: clip, too, unless we care about Safari <16, but I don't think we do since we specify last 3 safari versions in package.json.

An alternative could be to have overflow: clip as the default, but add a prop for overflow or something that allows consumers to switch between clip and hidden (and maybe any other overflow value?)

---

We use position: sticky on the Reports Index and I believe we're telling folks to use position: sticky instead of relying on a <Sticky /> component, so having this work by default in the most common use cases (e.g. like having a <LegacyCard /> container) seems important to me.

[mattkubej]

Good catch @nickpresta! That seems like it should be a better choice in this circumstance.

I'm going to update this PR and the spin instance, so it can be top hatted with clip to see if this has us covered.

[mattkubej]

/snapit

[github-actions]

🫰✨ Thanks @mattkubej! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris-cli@0.0.0-snapshot-release-20230530224523

yarn add @shopify/polaris@0.0.0-snapshot-release-20230530224523

[mattkubej]

/snapit

[github-actions]

🫰✨ Thanks @mattkubej! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris-cli@0.0.0-snapshot-release-20230530235250

yarn add @shopify/polaris@0.0.0-snapshot-release-20230530235250

[nickpresta]

Can confirm that the Reports Index sticky headers are working as expected:

[ouellettejordan]

Looks good to me 👍

[nickpresta]

👍 LGTM for Analytics