Skip to content

Conversation

@laurkim
Copy link
Contributor

@laurkim laurkim commented Sep 2, 2022

WHY are these changes introduced?

Resolves #7055.
Tests implementation of the alpha Box component in existing Polaris components.

WHAT is this pull request doing?

Uses Box in the Toast, ContextualSaveBar, and Banner components.

  • Removes all custom div/button/span elements from above components to use Box instead

Adds API support for:

  • button on the as prop in Box
  • className prop in Box and as a result, removes Box from exports to have it as an internal component
  • aria attributes
  • events (onBlur, onClick, onKeyUp, onMouseUp)
  • disabled state
  • id identifier
  • tabIndex
    Toast example using Box Toast example using Box
    ContextualSaveBar example using Box ContextualSaveBar example using Box
    Banner example using Box Banner example using Box

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React, {useState, useCallback} from 'react';

import {
  Page,
  Frame,
  Toast,
  Button,
  ContextualSaveBar,
  Link,
  Banner,
} from '../src';

export function Playground() {
  // Uncomment the below to test the Toast component
  // const [active, setActive] = useState(false);

  // const toggleActive = useCallback(() => setActive((active) => !active), []);

  // const toastMarkup = active ? (
  //   <Toast content="It's toasty" onDismiss={toggleActive} />
  // ) : null;

  return (
    // Uncomment the below to test the Toast component
    // <Frame>
    //   <Page title="Toast example using Box">
    //     <Button onClick={toggleActive}>Show Toast (with 📦)</Button>
    //     {toastMarkup}
    //   </Page>
    // </Frame>

    // Uncomment the below to test the ContextualSaveBar component
    // <Frame
    //   logo={{
    //     width: 124,
    //     contextualSaveBarSource:
    //       'https://cdn.shopify.com/s/files/1/0446/6937/files/jaded-pixel-logo-gray.svg?6215648040070010999',
    //   }}
    // >
    //   <ContextualSaveBar
    //     message="Unsaved changes"
    //     saveAction={{
    //       onAction: () => console.log('save action clicked'),
    //       loading: false,
    //       disabled: false,
    //     }}
    //     discardAction={{
    //       onAction: () => console.log('discard action clicked'),
    //     }}
    //   />
    // </Frame>

    // Uncomment the below to test the Banner component
    <Page>
      <Banner
        title="USPS has updated their rates"
        action={{content: 'Update rates', url: ''}}
        secondaryAction={{content: 'Learn more'}}
        status="info"
        onDismiss={() => {}}
      >
        <p>Make sure you know how these changes affect your store.</p>
      </Banner>
    </Page>
  );
}

🎩 checklist

@laurkim laurkim self-assigned this Sep 2, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2022

size-limit report 📦

Path Size
polaris-react-cjs 203.49 KB (+0.1% 🔺)
polaris-react-esm 130.41 KB (+0.12% 🔺)
polaris-react-esnext 185.24 KB (+0.1% 🔺)
polaris-react-css 41.12 KB (+0.02% 🔺)

@laurkim laurkim changed the title [Layout foundations] Prototype Box usage in existing components [Layout foundations][WIP] Prototype Box usage in existing components Sep 2, 2022
@laurkim laurkim force-pushed the layout-foundations-prototype branch 2 times, most recently from dc71d65 to a67605f Compare September 6, 2022 18:41
@laurkim laurkim force-pushed the lo/use-box-component branch 6 times, most recently from cdb7563 to ee0387b Compare September 8, 2022 15:17
@laurkim laurkim marked this pull request as ready for review September 8, 2022 17:12
@laurkim laurkim changed the title [Layout foundations][WIP] Prototype Box usage in existing components [Layout foundations] Prototype Box usage in existing components Sep 8, 2022
@laurkim laurkim force-pushed the lo/use-box-component branch 2 times, most recently from 193874a to 0661fce Compare September 8, 2022 18:53
@laurkim laurkim force-pushed the layout-foundations-prototype branch 3 times, most recently from 2375302 to b30c61c Compare September 13, 2022 17:32
@laurkim laurkim force-pushed the lo/use-box-component branch 2 times, most recently from 9027efe to 8290d00 Compare September 15, 2022 19:53
@laurkim
Copy link
Contributor Author

laurkim commented Sep 29, 2022

Closing this in favor of the second prototype where we explored using Box in Toast without the className prop on Box. We preferred that prototype over this one due to the complexity that having a className escape hatch would introduce to the component.

@laurkim laurkim closed this Sep 29, 2022
laurkim added a commit that referenced this pull request Sep 29, 2022
…7279)

### WHY are these changes introduced?

Resolves #7134.
[Storybook
URL](https://5d559397bae39100201eedc1-bongofonfc.chromatic.com/?path=/story/all-components-toast--default).

An earlier [prototype](#7096)
explored adding `className` support to the `Box` component and keeping
it as an internal component.
This prototype explores keeping `Box` external without the `className`
prop and replacing custom divs in `Toast` with the `Box` component.

Even if we don't update `Toast` yet, there are some commits in this PR
I'd like to keep that cleans up some of the types and prop descriptions
in the `Box` component.

### WHAT is this pull request doing?
- Cleans up types and updates prop descriptions in `Box`
- Adds `color` and `maxWidth` support to `Box`
- Updates `Toast` to use `Box` where custom `div` are being used

    <details>
      <summary>Toast original</summary>
<img
src="https://user-images.githubusercontent.com/26749317/192353275-5ca804db-0873-4e13-9061-59ee13425538.gif"
alt="Toast original">
    </details>
    <details>
      <summary>Toast refactored with Box</summary>
<img
src="https://user-images.githubusercontent.com/26749317/192353264-6eff870c-ddfb-4583-8d57-6a93f15daf94.gif"
alt="Toast refactored with Box">
    </details>

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React, {useState, useCallback} from 'react';

import {Page, Frame, Toast, Button} from '../src';

export function Playground() {
  const [active, setActive] = useState(false);

  const toggleActive = useCallback(() => setActive((active) => !active), []);

  const toastMarkup = active ? (
    <Toast content="It's toasty" onDismiss={toggleActive} />
  ) : null;

  return (
    <Frame>
      <Page title="Toast example using Box">
        <Button onClick={toggleActive}>Show Toast (with 📦)</Button>
        {toastMarkup}
      </Page>
    </Frame>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

Co-authored-by: Kyle Durand <[email protected]>
laurkim added a commit that referenced this pull request Sep 30, 2022
…7279)

### WHY are these changes introduced?

Resolves #7134.
[Storybook
URL](https://5d559397bae39100201eedc1-bongofonfc.chromatic.com/?path=/story/all-components-toast--default).

An earlier [prototype](#7096)
explored adding `className` support to the `Box` component and keeping
it as an internal component.
This prototype explores keeping `Box` external without the `className`
prop and replacing custom divs in `Toast` with the `Box` component.

Even if we don't update `Toast` yet, there are some commits in this PR
I'd like to keep that cleans up some of the types and prop descriptions
in the `Box` component.

### WHAT is this pull request doing?
- Cleans up types and updates prop descriptions in `Box`
- Adds `color` and `maxWidth` support to `Box`
- Updates `Toast` to use `Box` where custom `div` are being used

    <details>
      <summary>Toast original</summary>
<img
src="https://user-images.githubusercontent.com/26749317/192353275-5ca804db-0873-4e13-9061-59ee13425538.gif"
alt="Toast original">
    </details>
    <details>
      <summary>Toast refactored with Box</summary>
<img
src="https://user-images.githubusercontent.com/26749317/192353264-6eff870c-ddfb-4583-8d57-6a93f15daf94.gif"
alt="Toast refactored with Box">
    </details>

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React, {useState, useCallback} from 'react';

import {Page, Frame, Toast, Button} from '../src';

export function Playground() {
  const [active, setActive] = useState(false);

  const toggleActive = useCallback(() => setActive((active) => !active), []);

  const toastMarkup = active ? (
    <Toast content="It's toasty" onDismiss={toggleActive} />
  ) : null;

  return (
    <Frame>
      <Page title="Toast example using Box">
        <Button onClick={toggleActive}>Show Toast (with 📦)</Button>
        {toastMarkup}
      </Page>
    </Frame>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

Co-authored-by: Kyle Durand <[email protected]>
@laurkim laurkim deleted the lo/use-box-component branch November 15, 2023 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants