[Card] Fix padding in small devices

[amireynoso]

WHY are these changes introduced?

Resolves https://github.com/Shopify/polaris-react-deprecated/issues/1999

Card is supposed to have a 16px padding in small devices, as defined in the UI kit. This was causing inconsistencies when Card is used with other components that respect the 16px spacing rule [see issue for screenshots and live demo].

WHAT is this pull request doing?

Sets the current spacing rules (20px) as desktop rules using the page-content-when-not-fully-condensed breakpoint (already in place for Card). And sets 16px as the default value.

Questions / concerns

I'm using the page-content-when-not-fully-condensed mixin, which was already being used in Card (breakpoint is at around 490px).

However, it's not consistent to other breakpoints in components that should behave similarly, such as ResourceList (breakpoint is at 458px).

Is page-content-when-not-fully-condensed the right call in this case? Or should I use the same hard-coded value as ResourceList?

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Screenshots

Before fix

Notice the difference between the ResourceList spacing in the second card and the card title (mobile only).

After fix

Desktop stays the same.

Copy-paste this code in playground/Playground.tsx:

import * as React from 'react';
import {
  Page,
  Card,
  ResourceList,
  CalloutCard,
  TextStyle,
} from '@shopify/polaris';

export default class Playground extends React.Component<never, never> {
  render() {
    return (
      <Page title="Playground">
        <Card title="Online store dashboard">
          <Card.Section title="Reports">
            <p>View a summary of your online store’s performance.</p>
          </Card.Section>

          <Card.Section title="Summary">
            <p>
              View a summary of your online store’s performance, including
              sales, visitors, top products, and referrals.
            </p>
          </Card.Section>
        </Card>
        <Card title="Card title">
          <ResourceList
            resourceName={{singular: 'customer', plural: 'customers'}}
            items={[
              {
                id: 341,
                url: 'customers/341',
                name: 'Mae Jemison',
                location: 'Decatur, USA',
              },
              {
                id: 256,
                url: 'customers/256',
                name: 'Ellen Ochoa',
                location: 'Los Angeles, USA',
              },
            ]}
            renderItem={this.renderItem}
          />
        </Card>
        <CalloutCard
          title="Customize the style of your checkout"
          illustration="https://cdn.shopify.com/s/assets/admin/checkout/settings-customizecart-705f57c725ac05be5a34ec20c05b94298cb8afd10aac7bd9c7ad02030f48cfa0.svg"
          primaryAction={{
            content: 'Customize checkout',
            url: 'https://www.shopify.com',
          }}
        >
          <p>Upload your store’s logo, change colors and fonts, and more.</p>
        </CalloutCard>
      </Page>
    );
  }

  private renderItem(item: any) {
    const {id, url, name, location} = item;

    return (
      <ResourceList.Item
        id={id}
        url={url}
        accessibilityLabel={`View details for ${name}`}
      >
        <h3>
          <TextStyle variation="strong">{name}</TextStyle>
        </h3>
        <div>{location}</div>
      </ResourceList.Item>
    );
  }
}

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated UNRELEASED.md

[ghost]

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack.

[amireynoso]

On https://github.com/Shopify/polaris-react-deprecated/issues/1999#issuecomment-417046795 it was discussed that side paddings should be 16px, but for top and bottom, should it still be 20px?

[chloerice]

IMO yes, but tagging in @sarahill 👋

[chloerice]

I'm using the page-content-when-not-fully-condensed mixin, which was already being used in Card (breakpoint is at around 490px).

However, it's not consistent to other breakpoints in components that should behave similarly, such as ResourceList (breakpoint is at 458px).

Is page-content-when-not-fully-condensed the right call in this case? Or should I use the same hard-coded value as ResourceList?

I'm not sure what the basis for having the special breakpoint in the resource list component is, but you're using the correct mixin here 👍

[amireynoso]

Thanks, @chloerice!

I'll wait for @sarahill's input into what the top/bottom padding should be, and update if needed.

[chloerice]

Code and tophat are 👍

[sarahill]

This looks good to me! Thanks for making this fix 💯

[amireynoso]

Thanks, @sarahill and @chloerice! I've just updated the conflicts in the Changelog.

This is my first PR in Polaris, what are the next steps?

[chloerice]

This is my first PR in Polaris, what are the next steps?

Thanks again, @amireynoso 🎉 You're good to merge once the conflicts are resolved in UNRELEASED (it's an ever-moving target 😣). Ping me if you get any percy snapshot stuff that you're unsure of. (It's kind of cryptic, but for this PR the shift to the left of the red marks is expected)

[amireynoso]

Hi @chloerice, thank you so much for your help! Seconds before you commented, I resolved the conflicts in the UNRELEASED doc. It made the Percy check fail again. Would you mind re-approving? I'll merge afterwards. Thanks again! 🙌

[ghost]

🎉 Thanks for your contribution to Polaris React!