feat(ODD): add snackbars and toasts #12378

ewagoner · 2023-03-28T22:53:14Z

Overview

This PR adds Toasts and Snackbars to the ODD. It also refactored the ToasterOven component and hooks so it could bake both types of components.

snackbar: https://www.figma.com/file/OIdG64Q5cgvEw82ish5Eee/Design-System-(Flex)?node-id=1108-55875&t=9rfcF1oaxwy9W6vd-0

toasts: https://www.figma.com/file/OIdG64Q5cgvEw82ish5Eee/Design-System-(Flex)?node-id=1108-55574&t=BP86HBXk18gH2gFv-0

TODO: Add closing animation (also still a TODO for desktop toasts)

Closes RAUT-379

Test Plan

There are complete sets of storybook stories and unit tests for both components.

Changelog

Added Snackbar
Added Toast for the ODD
Refactored ToasterOven and moved it into /organisms
Added ToasterOven wrapper around ODD app

Review requests

The storybook stories have all the different styles and configurations.

Risk assessment

Low

Screen.Recording.2023-03-28.at.6.45.59.PM.mov

Screen.Recording.2023-03-28.at.6.46.40.PM.mov

codecov · 2023-03-28T22:59:03Z

Codecov Report

Merging #12378 (528464b) into edge (82fd757) will decrease coverage by 0.03%.
The diff coverage is 77.01%.

@@            Coverage Diff             @@
##             edge   #12378      +/-   ##
==========================================
- Coverage   73.81%   73.79%   -0.03%     
==========================================
  Files        2206     2229      +23     
  Lines       60935    61732     +797     
  Branches     6219     6436     +217     
==========================================
+ Hits        44981    45554     +573     
- Misses      14478    14671     +193     
- Partials     1476     1507      +31

Flag	Coverage Δ
app	`72.30% <76.74%> (-0.19%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
app/src/App/DesktopApp.tsx	`100.00% <ø> (ø)`
...p/src/organisms/Devices/hooks/useDownloadRunLog.ts	`0.00% <0.00%> (ø)`
...DeviceDisplay/ProtocolDashboard/LongPressModal.tsx	`48.57% <ø> (ø)`
app/src/organisms/ToasterOven/ToasterOven.tsx	`27.77% <33.33%> (ø)`
app/src/atoms/Toast/index.tsx	`76.19% <76.19%> (ø)`
app/src/App/OnDeviceDisplayApp.tsx	`61.53% <100.00%> (ø)`
app/src/atoms/Snackbar/index.tsx	`100.00% <100.00%> (ø)`
...ices/RobotSettings/AdvancedTab/Troubleshooting.tsx	`95.00% <100.00%> (ø)`
app/src/organisms/ModuleCard/index.tsx	`62.76% <100.00%> (ø)`
.../src/organisms/SendProtocolToOT3Slideout/index.tsx	`63.63% <100.00%> (ø)`
... and 5 more

... and 43 files with indirect coverage changes

shlokamin

wondering if we should name ToasterOven something more generic now that it can render toasts + snacks.

thought for a moment about having them be two different providers, but i think we want them to be shared by the same "oven" so they dont stomp on each other.

app/src/atoms/Toast/index.tsx

shlokamin · 2023-03-29T14:32:34Z

app/src/atoms/Toast/index.tsx

@@ -34,9 +36,10 @@ export type ToastType =
 export interface ToastProps extends StyleProps {
 id: string
 message: string
+ secondaryMessage?: string


is secondaryMessage being used anywhere?

Yes. It's used throughout the ODD designs to pass protocol names where needed.

same comment as above re: sharing types. as far as i see the desktop toast doesn't use secondaryMessage

It doesn't yet but the design spec in Figma says "Note in the future: Toast and inline toast are the same component in desktop DS" which I interpreted to mean she would like the component to be shared between apps, using new design and props. I was trying to lay the groundwork for that by sharing as much as possible now so only the HTML will have to be swapped out later. I can totally make them two separate things though and someone later can put them together.

app/src/organisms/ToasterOven/ToasterOven.tsx

shlokamin · 2023-03-29T14:45:58Z

also, getting an error when hitting this storybook link: https://s3-us-west-2.amazonaws.com/opentrons-components/RAUT-379-odd-snackbar/index.html?path=/story/odd-atoms-snackbar--quick-snack

ewagoner · 2023-03-29T16:01:06Z

also, getting an error when hitting this storybook link

That's from the button import looking in the wrong place after that recent component work. I pushed a fix to my branch yesterday but maybe storybook didn't get rebuilt? I'll look at that. It works locally for me.

ewagoner · 2023-03-29T18:28:21Z

wondering if we should name ToasterOven something more generic

Open to name suggestions. I thought about changing it to something boring like NotificationProvider or something like that but then realized ToasterOven is an even better name than it was before, because now it can both make toast and bake snacks :)

app/src/atoms/Toast/OnDeviceDisplay/Toast.stories.tsx

app/src/atoms/Toast/OnDeviceDisplay/Toast.tsx

app/src/atoms/Toast/Toast.stories.tsx

brenthagen · 2023-03-29T18:57:52Z

app/src/atoms/Toast/OnDeviceDisplay/Toast.tsx

+ },
+}
+
+export function Toast(props: ToastProps): JSX.Element {


to avoid confusion, i think this component should be explicitly named ODDToast or similar

Ok. We're not doing that for the other components (see buttons, etc.) where we made ODD versions to replace desktop components. I am importing it into the ToasterOven with the ODD prefix to make it clear there:

import { Toast as ODDToast } from '../../atoms/Toast/OnDeviceDisplay/Toast'

I can just rename it though.

to clarify, my concern is not the ODD prefix, but confusion over different components with exactly the same name

let's talk this through in the app & ui channel (ill start a message), i have thoughts but want other folks to be part of this discussion

It was easy to rename it and separate out the props and such so the two are totally separate entities. The ToasterOven is able to deal with whichever type comes through it. I did that work before seeing this, and can adapt it to however that discussion turns out.

https://opentrons.slack.com/archives/CSCLVUW3C/p1680121443070599

brenthagen · 2023-03-29T19:00:16Z

app/src/organisms/ToasterOven/ToasterContext.ts

+
+type MakeSnackbar = (message: string, options?: MakeSnackbarOptions) => void
+
+type EatSnackbar = () => void


what's the use case for eatSnackbar? is the snackbar ever intended to close based on something other than a timeout? eatToast exist to programmatically close based on some event that happens asynchronously

I'm not using it anywhere yet, but really the only use case is forcing a snackbar to close prematurely so another one could take its place -- there is a limit of one snackbar displayed at a time.

I took it out. It's premature optimization and one less thing to mock in tests that want to use the ToasterOven but don't care about snacks.

Noting for posterity that it was actually essential for removing the snackbar from the HTML page when used by the ToasterOven, so this will be making a return in a future PR. In isolation, it closes itself (in tests and storybook) but when placed on a page by the app, it has to be removed by force via an eat method.

app/src/organisms/ToasterOven/ToasterOven.tsx

…RAUT-379]

]

ewagoner · 2023-03-31T13:37:52Z

Moving this back into Draft while I apply the design refactoring that better combines Desktop and ODD toasts into a single skinnable component.

ewagoner · 2023-03-31T15:07:17Z

Ok! I've incorporated the new design and re-unified the props and component. You'll see I cheated just a little because we do want to unit test and storybook the two differently, so the toast component exports two versions that can be imported into test files and storybook stories separately.

ewagoner · 2023-03-31T20:41:24Z

Pulling back into draft mode to implement some design tweaks hashed out with Shlok, Mel, and Koji.

ewagoner · 2023-04-01T21:17:13Z

@shlokamin @koji @mmencarelli I've applied the changes we discussed and reduced the two components down to a single Toast (with one additional abstraction only to account for the useSelector call that we can't do inside storybook).

koji · 2023-04-01T22:46:23Z

@ewagoner
I updated your message because you mentioned different Mel who isn't in app & ui team lol

shlokamin

this is great, and gets us much much closer to the desired architecture i described in the comment below. thank you!

I scheduled time for us to talk with mel about a naming system to be able to move forward with the proposed architecture.

Two things:

take a look at the storybook guidelines i wrote on Friday afternoon (very new). mel wants us to have one single story with configurable props
there is one place in the toast component where we swap a whole JSX element rather than just swapping styles. we'll need to refactor that (assuming that is possible...) in order to make the proposed design token architecture work

im fine with this merging and addressing the things above in a follow up, whatever works best for you!

shlokamin · 2023-04-03T18:15:10Z

app/src/atoms/Toast/index.tsx

+// I really, really wanted to get this done in a single component, but the useSelector
+// is a huuuuge pain to try to get to work in storybook. I couldn't mock it, I couldn't
+// ignore it, and I couldn't get storybook to work in isolation with it there. So... I
+// confined it to the Toast omponent the rest of the app sees and have it determine what
+// display it is using at runtime. Storybook then can use the RawToast and just pass in
+// what display it wants to see.


that is very annoying. i think a good path forward to get around this is to leverage some sort of theme provider like this that we'd make on our own.

we could define it and pull it in at the top level of our app like we do for redux + react router + react query + i18n: https://github.com/Opentrons/opentrons/blob/edge/app/src/index.tsx

then inside of our tests, we can have our renderwithProviders util take in an additional option flag that tells our testing environ,ent which theme provider to use (we can default to desktop app). okay... that takes care of the testing hurdle.

as for storybook, its kinda similar. inside of our stories we can render each ODD component inside of the ODD theme provider like this:

const MediumButtonTemplate: Story< React.ComponentProps<typeof MediumButton> > = args => <ThemeProvider theme="ODD" ><MediumButton {...args} /><ThemeProvider theme="ODD" >

okay... but here come's the hard part. what do with all of the ternaries?? we have to come up with some sort of semantic variable that describes a color that is shared across platforms. ex.

backgroundColor: `${ showODDStyle ? COLORS.yellow_four : COLORS.errorBackgroundLight }`, // would need to become backgroundColor: `${COLORS. errorYellow}`,

and COLORS.errorYellow would be provided by the theme provider depending on which platform we're on.

that means we have to come up with a strict mapping of platform to colors, and name them something semantically meaningful. this will be difficult and require close collaboration with @mmencarelli

how does that sound?

app/src/atoms/Toast/index.tsx

ewagoner · 2023-04-03T18:46:11Z

take a look at the storybook guidelines i wrote on Friday afternoon (very new). mel wants us to have one single story with configurable props

@shlokamin Those guidelines say If the component has a version that runs on both desktop and ODD, there should be a story for each platform. This is what I've done. Are you saying they should be combined into a single story?

shlokamin · 2023-04-03T18:54:02Z

in Toast.stories.tsx i see 8 stories:

export const Success = TemplateWithTimeout.bind({})
Success.args = {
  message: 'Success Toast message',
  type: 'success',
}

export const Warning = TemplateWithTimeout.bind({})
Warning.args = {
  message: 'Warning Toast message',
  type: 'warning',
}

export const Error = TemplateWithTimeout.bind({})
Error.args = {
  message: 'Error Toast message',
  type: 'error',
}

export const Info = TemplateWithTimeout.bind({})
Info.args = {
  message: 'Info Toast message',
  type: 'info',
}


export const SuccessWithoutTimeout = TemplateWithoutTimeout.bind({})
SuccessWithoutTimeout.args = {
  message: 'Success Toast message',
  type: 'success',
  disableTimeout: true,
}

export const WarningWithoutTimeout = TemplateWithoutTimeout.bind({})
WarningWithoutTimeout.args = {
  message: 'Warning Toast message',
  type: 'warning',
  disableTimeout: true,
}

export const ErrorWithoutTimeout = TemplateWithoutTimeout.bind({})
ErrorWithoutTimeout.args = {
  message: 'Error Toast message',
  type: 'error',
  disableTimeout: true,
}

export const InfoWithoutTimeout = TemplateWithoutTimeout.bind({})
InfoWithoutTimeout.args = {
  message: 'Info Toast message',
  type: 'info',
  disableTimeout: true,
}

and in ODDToast.stories.tsx i see another 8:

export const Success = TemplateWithTimeout.bind({})
Success.args = {
  message: 'Success Toast message',
  type: 'success',
}

export const Warning = TemplateWithTimeout.bind({})
Warning.args = {
  message: 'Warning Toast message',
  type: 'warning',
}

export const Error = TemplateWithTimeout.bind({})
Error.args = {
  message: 'Error Toast message',
  type: 'error',
}

export const Info = TemplateWithTimeout.bind({})
Info.args = {
  message: 'Info Toast message',
  type: 'info',
}


export const SuccessWithoutTimeout = TemplateWithoutTimeout.bind({})
SuccessWithoutTimeout.args = {
  message: 'Success Toast message',
  type: 'success',
  disableTimeout: true,
}

export const WarningWithoutTimeout = TemplateWithoutTimeout.bind({})
WarningWithoutTimeout.args = {
  message: 'Warning Toast message',
  type: 'warning',
  disableTimeout: true,
}

export const ErrorWithoutTimeout = TemplateWithoutTimeout.bind({})
ErrorWithoutTimeout.args = {
  message: 'Error Toast message',
  type: 'error',
  disableTimeout: true,
}

export const InfoWithoutTimeout = TemplateWithoutTimeout.bind({})
InfoWithoutTimeout.args = {
  message: 'Info Toast message',
  type: 'info',
  disableTimeout: true,
}

shlokamin · 2023-04-03T18:55:03Z

we want one ODD toast story and one desktop story with configurable props

ewagoner · 2023-04-03T18:59:41Z

we want one ODD toast story and one desktop story with configurable props

Oh, I see what you mean. I was counting the whole file as one story, and it contained multiple examples of how the configurable props could be initialized, so you could easily see them in action and set them as you want. I'll remove all the examples.

app/src/atoms/Toast/index.tsx

]

ewagoner · 2023-04-04T03:17:30Z

Two things:

@shlokamin I managed to address both things. The storybook stories have been simplified and just contain a single example and the Flex component swapping has been reduced to pure CSS setting.

ewagoner requested review from koji, jerader and a team March 28, 2023 22:55

ewagoner force-pushed the RAUT-379-odd-snackbar branch from eec5bbc to 9a6a74a Compare March 28, 2023 23:02

ewagoner marked this pull request as ready for review March 28, 2023 23:03

ewagoner requested a review from a team as a code owner March 28, 2023 23:03

shlokamin requested review from mmencarelli and brenthagen March 29, 2023 14:16

shlokamin reviewed Mar 29, 2023

View reviewed changes

koji reviewed Mar 29, 2023

View reviewed changes

app/src/atoms/Toast/OnDeviceDisplay/Toast.stories.tsx Outdated Show resolved Hide resolved

ewagoner added 9 commits March 29, 2023 14:41

Create snackbar component, stories, and tests [RAUT-379]

e71d113

Roll snackbar into ToastContext [RAUT-379]

4e12a41

Add ToasterOven to ODD app [RAUT-379]

75c462c

Move ToasterOven from atoms to organisms [RAUT-379]

fb0baa1

Add beginnings of ODD styling to toast [RAUT-379]

d6353b9

Separate out ODD toast component and add stories and tests [RAUT-379]

d7744bc

Removed manual test code [RAUT-379]

9b8ebe9

Fixed typo in storybook names [RAUT-379]

2c45325

Fixed stories to use new button location [RAUT-379]

114a911

koji reviewed Mar 29, 2023

View reviewed changes

app/src/atoms/Toast/OnDeviceDisplay/Toast.tsx Outdated Show resolved Hide resolved

koji reviewed Mar 29, 2023

View reviewed changes

app/src/atoms/Toast/OnDeviceDisplay/Toast.tsx Outdated Show resolved Hide resolved

koji reviewed Mar 29, 2023

View reviewed changes

app/src/atoms/Toast/Toast.stories.tsx Outdated Show resolved Hide resolved

Initialize snackbar as null instead of undefined [RAUT-379]

7448e6a

ewagoner force-pushed the RAUT-379-odd-snackbar branch from 82a9bb4 to 7448e6a Compare March 29, 2023 18:56

brenthagen reviewed Mar 29, 2023

View reviewed changes

app/src/organisms/ToasterOven/ToasterOven.tsx Outdated Show resolved Hide resolved

ewagoner requested review from shlokamin and koji March 29, 2023 21:03

Separate Toasts and ODDToasts as well as other more minor PR feedback […

447d612

…RAUT-379]

ewagoner force-pushed the RAUT-379-odd-snackbar branch from 54713d0 to 447d612 Compare March 29, 2023 21:26

Added variable duration for toast closure based on text length [RAUT-379

eb07b2a

]

ewagoner marked this pull request as draft March 31, 2023 13:38

Desktop and ODD Toast reunification and reconciliation [RAUT-379]

2262526

ewagoner marked this pull request as ready for review March 31, 2023 15:07

ewagoner marked this pull request as draft March 31, 2023 20:40

Further reunification of the ODD and Desktop Toast components [RAUT-379]

a8cb094

ewagoner marked this pull request as ready for review April 1, 2023 21:15

shlokamin approved these changes Apr 3, 2023

View reviewed changes

koji reviewed Apr 3, 2023

View reviewed changes

app/src/atoms/Toast/index.tsx Outdated Show resolved Hide resolved

ewagoner added 2 commits April 3, 2023 22:57

Another round of component consolidation and storybook writing [RAUT-379

f253a49

]

Further storybook refinement [RAUT-379]

88287fd

Use translation for default close button text [RAUT-379]

528464b

ewagoner requested review from shlokamin and koji April 4, 2023 13:11

ewagoner merged commit db8929c into edge Apr 4, 2023

ewagoner deleted the RAUT-379-odd-snackbar branch April 4, 2023 19:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ODD): add snackbars and toasts #12378

feat(ODD): add snackbars and toasts #12378

ewagoner commented Mar 28, 2023 •

edited

Loading

codecov bot commented Mar 28, 2023 •

edited

Loading

shlokamin left a comment

shlokamin Mar 29, 2023

ewagoner Mar 29, 2023

brenthagen Mar 29, 2023

ewagoner Mar 29, 2023 •

edited

Loading

shlokamin commented Mar 29, 2023 •

edited

Loading

ewagoner commented Mar 29, 2023

ewagoner commented Mar 29, 2023

brenthagen Mar 29, 2023

ewagoner Mar 29, 2023

brenthagen Mar 29, 2023

shlokamin Mar 29, 2023

ewagoner Mar 29, 2023

shlokamin Mar 29, 2023

brenthagen Mar 29, 2023

ewagoner Mar 29, 2023 •

edited

Loading

ewagoner Mar 29, 2023

ewagoner Apr 4, 2023

ewagoner commented Mar 31, 2023

ewagoner commented Mar 31, 2023

ewagoner commented Mar 31, 2023

ewagoner commented Apr 1, 2023 •

edited by koji

Loading

koji commented Apr 1, 2023

shlokamin left a comment

shlokamin Apr 3, 2023

ewagoner commented Apr 3, 2023

shlokamin commented Apr 3, 2023

shlokamin commented Apr 3, 2023

ewagoner commented Apr 3, 2023 •

edited

Loading

ewagoner commented Apr 4, 2023


		type MakeSnackbar = (message: string, options?: MakeSnackbarOptions) => void

		type EatSnackbar = () => void

feat(ODD): add snackbars and toasts #12378

feat(ODD): add snackbars and toasts #12378

Conversation

ewagoner commented Mar 28, 2023 • edited Loading

Overview

Test Plan

Changelog

Review requests

Risk assessment

codecov bot commented Mar 28, 2023 • edited Loading

Codecov Report

shlokamin left a 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

ewagoner Mar 29, 2023 • edited Loading

Choose a reason for hiding this comment

shlokamin commented Mar 29, 2023 • edited Loading

ewagoner commented Mar 29, 2023

ewagoner commented Mar 29, 2023

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

ewagoner Mar 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewagoner commented Mar 31, 2023

ewagoner commented Mar 31, 2023

ewagoner commented Mar 31, 2023

ewagoner commented Apr 1, 2023 • edited by koji Loading

koji commented Apr 1, 2023

shlokamin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewagoner commented Apr 3, 2023

shlokamin commented Apr 3, 2023

shlokamin commented Apr 3, 2023

ewagoner commented Apr 3, 2023 • edited Loading

ewagoner commented Apr 4, 2023

ewagoner commented Mar 28, 2023 •

edited

Loading

codecov bot commented Mar 28, 2023 •

edited

Loading

ewagoner Mar 29, 2023 •

edited

Loading

shlokamin commented Mar 29, 2023 •

edited

Loading

ewagoner Mar 29, 2023 •

edited

Loading

ewagoner commented Apr 1, 2023 •

edited by koji

Loading

ewagoner commented Apr 3, 2023 •

edited

Loading