fix(Toast): UXD-1237 Refactor toast internal states and fix canAutoClose for controlled #1079

josh-luong · 2021-06-10T22:33:18Z

Purpose 🚀

Refactor how the internal state is coupled when using the Toast as a controlled or uncontrolled component.

Notes ✏️

In the current toast, the internal states isToastOpen, shouldRender was overcomplicated and coupled with the parent prop isOpen when it was used as a controlled component. As a result, this would lead to some undesired behaviour such as canAutoClose breaking the controlled toast.
- Issue example: https://codesandbox.io/s/heuristic-kepler-fclbd?file=/src/App.js
Updated behaviour such that the internal state isToastOpen solely handles the render flow of the toast, and is toggled by isOpen when controlled.
Updated documentation and added a story to exemplify when the consumer wants an uncontrolled toast that can be rerendered by updating the component's key to reset its internal state.
- In reference to issue: Using same Toast multiple times with canAutoClose #1016

Storybook 📕

http://storybooks.highbond-s3.com/paprika/UXD-1177-refactor-toast-component

References 🔗

relevant Jira ticket / GitHub issues
https://aclgrc.atlassian.net/browse/UXD-1177
https://aclgrc.atlassian.net/browse/UXD-1136
#1016

Coauthor: @josh-luong @leonatgalvanize

…led and uncontrolled

…rvices/paprika into UXD-1177-refactor-toast-component

changeset-bot · 2021-06-10T22:33:21Z

🦋 Changeset detected

Latest commit: 56cab33

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@paprika/toast	Patch
@paprika/copy-input	Patch
@paprika/data-header	Patch
@paprika/list-box	Patch
@paprika/inline-editors	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

nahumzs · 2021-06-10T23:07:06Z

packages/Toast/README.md

 ```

+Note: An uncontrolled Toast is expected to be displayed and opened once, if the desired behavior is to display the Toast more than once, an alternative method is to reset the Toast by updating its key or use a controlled Toast component instead.
+


nahumzs · 2021-06-11T17:05:05Z

packages/Toast/README.md

@@ -25,10 +25,10 @@ npm install @paprika/toast
 | Prop           | Type                                                                                                                                                            | required | default               | Description                                                                                                                         |
 | -------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | --------------------- | ----------------------------------------------------------------------------------------------------------------------------------- |
 | autoCloseDelay | number                                                                                                                                                          | false    | 5000                  | Duration (in ms) before Toast will automatically close (if canAutoClose is true).                                                   |
-| canAutoClose   | bool                                                                                                                                                            | false    | false                 | Will automatically close after 1500ms (or longer if provided by autoCloseDelay).                                                    |
+| canAutoClose   | bool                                                                                                                                                            | false    | false                 | Will automatically close after 5000ms (or longer if provided by autoCloseDelay).                                                    |


is 5000ms a new value? was this suggested by a ux-design :) how did we arrive at this value?

The default value for autoCloseDelay was always 5000ms, so technically the default behaviour was that, so I updated the docs to reflect that. However, there is a 'minimumCloseTime' of 1500ms.

Right now the timeout timer is set by Math.max(autoCloseDelay, minimumCloseTime).

mikrotron · 2021-06-23T20:38:32Z

packages/Toast/stories/examples/UncontrolledToastExample.js

      <Toast hasCloseButton>Uncontrolled toast component</Toast>
      <Toast canAutoClose autoCloseDelay={5000} hasCloseButton>


A little out of scope, but while we're here, I think it makes sense to clean up this story just a little. There are some default props added here that are not actually needed, which is a distraction from the use case being illustrated.

mikrotron · 2021-06-23T20:43:49Z

packages/Toast/stories/examples/UncontrolledToastExample.js

  return (
    <L10n locale="en">
      <Toast hasCloseButton>Uncontrolled toast component</Toast>
      <Toast canAutoClose autoCloseDelay={5000} hasCloseButton>
        Uncontrolled toast component with auto close
      </Toast>
+      <CodeViewer>


While I appreciate the creative use of the <CodeViewer> here, I think maybe it would be more useful to just link to the whole story code on GitHub – a pattern established in several other components stories (<Input> is one example you can reference). Essentially by using the <ExampleStory> helper we can include a source link in the header.

mikrotron · 2021-06-23T20:44:26Z

packages/Toast/stories/examples/UncontrolledToastExample.js

+        <Toast key={accumulator} canAutoClose autoCloseDelay={5000}>
+          Uncontrolled toast with auto close and ability to re-open upon button click
+        </Toast>
+        <button type="button" onClick={handleItem}>


Minor suggestion: the story would be a little nicer looking with a Paprika <Button>

packages/Toast/src/Toast.js

nahumzs · 2021-07-08T18:13:23Z

packages/Toast/src/Toast.js

@@ -48,23 +48,34 @@ const Toast = React.forwardRef((props, ref) => {
    },
  }));

-  const [isToastOpen, setIsToastOpen] = React.useState(isOpen === undefined ? true : isOpen);
+  const isControlled = isOpen !== undefined;


typeof isOpen === "undefined" is a better approach readmore

interesting!

nahumzs · 2021-07-08T20:23:58Z

Lets merge this one if you think is at enough good stage @josh-luong, I think this branch has been running for more than a month, and the work is good enough for this iteration. As long as test are passing I'm 👌 with all this work.

trtri2 and others added 4 commits June 10, 2021 13:51

fix(Toast): UXD-1177 refactor internal toast states

5ea44ad

chore(Toast): UXD-1177 Update documentation

c0c93dc

chore(Toast): UXD-1177: adding stories for Toast canAutoClose control…

f1c1d9e

…led and uncontrolled

Merge branch 'UXD-1177-refactor-toast-component' of github.com:acl-se…

dab8ab0

…rvices/paprika into UXD-1177-refactor-toast-component

leonatgalvanize mentioned this pull request Jun 10, 2021

fix(Toast): UXD-1177 Refactor toast internal states and fix canAutoClose for controlled #1078

Closed

josh-luong and others added 2 commits June 10, 2021 15:36

chore(Toast): UXD-1237 UXD-1177: adding changeset

bacebc9

Merge branch 'master' into UXD-1237-refactor-toast-component

6a1badf

nahumzs reviewed Jun 10, 2021

View reviewed changes

nahumzs reviewed Jun 11, 2021

View reviewed changes

trtri2 and others added 3 commits June 11, 2021 14:22

docs(Toast): UXD-1237 update canAutoClose prop description

a91cf43

Merge branch 'master' into UXD-1237-refactor-toast-component

d5aca9b

Merge branch 'master' into UXD-1237-refactor-toast-component

50137a0

mikrotron reviewed Jun 23, 2021

View reviewed changes

packages/Toast/src/Toast.js Outdated Show resolved Hide resolved

josh-luong added 3 commits July 8, 2021 10:39

chore(Toast): [UXD-1237] updating stories

433fc62

fix(Toast): [UXD-1237] reverting to undefined isOpen prop

6633a58

fix(Toast): [UXD-1237] fixing isOpen conditions

090654f

nahumzs reviewed Jul 8, 2021

View reviewed changes

fix(Toast): [UXD-1237] fixing isOpen === undefined condition

163988c

nahumzs approved these changes Jul 8, 2021

View reviewed changes

Merge branch 'master' into UXD-1237-refactor-toast-component

56cab33

leonatgalvanize merged commit 9e9c692 into master Jul 8, 2021

leonatgalvanize deleted the UXD-1237-refactor-toast-component branch July 8, 2021 20:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Toast): UXD-1237 Refactor toast internal states and fix canAutoClose for controlled #1079

fix(Toast): UXD-1237 Refactor toast internal states and fix canAutoClose for controlled #1079

josh-luong commented Jun 10, 2021

changeset-bot bot commented Jun 10, 2021 •

edited

Loading

nahumzs Jun 10, 2021

nahumzs Jun 11, 2021

leonatgalvanize Jun 11, 2021

mikrotron Jun 23, 2021

mikrotron Jun 23, 2021

mikrotron Jun 23, 2021

nahumzs Jul 8, 2021

josh-luong Jul 8, 2021

nahumzs commented Jul 8, 2021

		```

		Note: An uncontrolled Toast is expected to be displayed and opened once, if the desired behavior is to display the Toast more than once, an alternative method is to reset the Toast by updating its key or use a controlled Toast component instead.

		<Toast hasCloseButton>Uncontrolled toast component</Toast>
		<Toast canAutoClose autoCloseDelay={5000} hasCloseButton>

fix(Toast): UXD-1237 Refactor toast internal states and fix canAutoClose for controlled #1079

fix(Toast): UXD-1237 Refactor toast internal states and fix canAutoClose for controlled #1079

Conversation

josh-luong commented Jun 10, 2021

Purpose 🚀

Notes ✏️

Storybook 📕

References 🔗

changeset-bot bot commented Jun 10, 2021 • edited Loading

🦋 Changeset detected

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nahumzs commented Jul 8, 2021

changeset-bot bot commented Jun 10, 2021 •

edited

Loading