[NO-JIRA] Migrate Floating Notification component to TS #3221
Conversation
|
Visit https://backpack.github.io/storybook-prs/3221 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3221 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3221 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3221 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3221 to see this build running in a browser. |
packages/bpk-component-floating-notification/src/BpkFloatingNotification.tsx
Outdated
Show resolved
Hide resolved
|
Visit https://backpack.github.io/storybook-prs/3221 to see this build running in a browser. |
| @@ -3,7 +3,7 @@ | |||
| exports[`BpkFloatingNotification should render correctly 1`] = ` | |||
| <DocumentFragment> | |||
| <div | |||
| class="bpk-floating-notification bpk-floating-notification--appear bpk-floating-notification--appear-active" | |||
| class="bpk-floating-notification" | |||
There was a problem hiding this comment.
Do you know why the classnames have changed here? Looks like it might not be appearing anymore?
There was a problem hiding this comment.
Interesting 👀 The UI behaviour was as expected but, for some reason, the tests were rendering the component without those classes. After some debugging, I found out that it was because of this change:
import { CSSTransition } from 'react-transition-group';
___________________________________________________________________
import CSSTransition from 'react-transition-group/CSSTransition';
It might be something related to babel and jest... Anyway, I already fixed it and rollbacked the snapshot changes 🙂
| @@ -30,7 +31,7 @@ const props = { | |||
|
|
|||
| describe('BpkFloatingNotification accessibility tests', () => { | |||
| it('should not have programmatically-detectable accessibility issues', async () => { | |||
| const { container } = render(<BpkFloatingNotification />); | |||
| const { container } = render(<BpkFloatingNotification {...props} />); | |||
There was a problem hiding this comment.
Just want to clarify the addition of the ...props here?
There was a problem hiding this comment.
The text prop is required in the component, calling it without it would fail.
I added the whole props object since it only has the text property.
|
Visit https://backpack.github.io/storybook-prs/3221 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3221 to see this build running in a browser. |
Description
Isotopes squad started to use the Floating Notification component for SAM and decided to contribute migrating this component to TS.
Remember to include the following changes:
[KOA-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md