Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript migration of snackbar stories #808

Merged
merged 6 commits into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 0 additions & 103 deletions apps/storybook-react/stories/Snackbar.stories.jsx

This file was deleted.

64 changes: 64 additions & 0 deletions apps/storybook-react/stories/Snackbar.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import React, { useState } from 'react'
import { Snackbar, SnackbarProps, Button } from '@equinor/eds-core-react'
import { Story, Meta } from '@storybook/react'

const { SnackbarAction } = Snackbar

export default {
title: 'Components/Snackbar',
component: Snackbar,
subcomponents: { SnackbarAction },
} as Meta

export const Default: Story<SnackbarProps> = (args) => {
const { open } = args
const [visible, setVisible] = useState(open)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove const { open } = args here and let the user trigger only at button click, that way the user can play with the other Snackbar examples too without interference... But it's weird how we get errors on it because this method has worked before 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the my main issue - how do we know that we are not just masking errors by just removing code that doesn't work? And then I'll have to turn off the control for open as well 😬

For the record, I'm doing the same thing with the Progress indicators - but because they is a single component with a single type it just works. But obviously not with the composed component approach

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open is right there!
Screenshot 2020-11-03 at 08 34 45
🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also did it on Popover with subcomponents

return (
<>
<Button type="button" onClick={() => setVisible(true)}>
Show a simple snackbar with default options
</Button>
<Snackbar {...args} open={visible} onClose={() => setVisible(false)}>
Play with me
</Snackbar>
</>
)
}
export const Simple: Story<SnackbarProps> = () => {
const [open, setOpen] = useState(false)

return (
<>
<Button type="button" onClick={() => setOpen(true)}>
Show a simple snackbar for 5 seconds
</Button>
<Snackbar
open={open}
onClose={() => setOpen(false)}
autoHideDuration={5000}
leftAlignFrom="1500px"
>
Message goes here
</Snackbar>
</>
)
}

export const WithAction: Story<SnackbarProps> = () => {
const [withActionOpen, setWithActionOpen] = useState(false)
return (
<>
<Button type="button" onClick={() => setWithActionOpen(true)}>
Show a snackbar with action for the default 7 seconds
</Button>
<Snackbar open={withActionOpen} onClose={() => setWithActionOpen(false)}>
Your changes was saved
<SnackbarAction>
<Button variant="ghost">Undo</Button>
</SnackbarAction>
</Snackbar>
</>
)
}

WithAction.storyName = 'With action'
19 changes: 10 additions & 9 deletions libraries/core-react/src/Snackbar/Snackbar.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useEffect, HTMLAttributes } from 'react'
import React, { useState, useEffect, HTMLAttributes, FC } from 'react'
import styled from 'styled-components'
import { snackbar as tokens } from './Snackbar.tokens'
import { typographyTemplate } from '../_common/templates'
Expand All @@ -23,6 +23,7 @@ const StyledSnackbar = styled.div.attrs(() => ({
box-sizing: border-box;
left: 50%;
transform: translateX(-50%);
z-index: 10;
@media (min-width: ${({ leftAlignFrom }) => leftAlignFrom}) {
left: auto;
transform: none;
Expand All @@ -34,25 +35,25 @@ const StyledSnackbar = styled.div.attrs(() => ({
}
`

type Props = {
/* Controls the visibility of the snackbar */
export type SnackbarProps = {
/** Controls the visibility of the snackbar */
open?: boolean
/* How long will the message be visible in milliseconds */
/** How long will the message be visible in milliseconds */
autoHideDuration?: number
/* Callback fired when the snackbar is closed by auto hide duration timeout */
/** Callback fired when the snackbar is closed by auto hide duration timeout */
onClose?: () => void
/* Media query from which the snackbar will be horizontal centered */
/** Media query from which the snackbar will be horizontal centered */
leftAlignFrom?: string
} & HTMLAttributes<HTMLDivElement>

export const Snackbar = ({
export const Snackbar: FC<SnackbarProps> = ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wenche Why FC here? I thought we should use forwardRef and named functions instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See commit message. It didn't have forwardRef and adding it will be a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh.. So we should add FC to those who didn't have forwardRef? What breaking change will adding forwardRef make? I'm pretty sure I've added forwardRef's to some of my components so I should probably go back to do it your way.. 😬

open = false,
autoHideDuration = 7000,
onClose,
leftAlignFrom = '1200px',
children,
className,
}: Props): JSX.Element => {
}) => {
const [visible, setVisible] = useState(open)
useEffect(() => {
setVisible(open)
Expand All @@ -75,4 +76,4 @@ export const Snackbar = ({
)
}

// Snackbar.displayName = 'eds-snackbar'
// Snackbar.displayName = 'Snackbar'
6 changes: 4 additions & 2 deletions libraries/core-react/src/Snackbar/SnackbarAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ const StyledSnackbarAction = styled.div`
margin-bottom: -10px;
`

type Props = {
type SnackbarActionProps = {
children: ReactNode
}

export const SnackbarAction = ({ children }: Props): JSX.Element => {
export const SnackbarAction = ({
children,
}: SnackbarActionProps): JSX.Element => {
return <StyledSnackbarAction>{Children.only(children)}</StyledSnackbarAction>
}
3 changes: 2 additions & 1 deletion libraries/core-react/src/Snackbar/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { SnackbarAction } from './SnackbarAction'
import { Snackbar as BaseComponent } from './Snackbar'
import { Snackbar as BaseComponent, SnackbarProps } from './Snackbar'

type SnackbarTypes = typeof BaseComponent & {
SnackbarAction: typeof SnackbarAction
Expand All @@ -10,3 +10,4 @@ const Snackbar = BaseComponent as SnackbarTypes
Snackbar.SnackbarAction = SnackbarAction

export { Snackbar }
export type { SnackbarProps }
2 changes: 1 addition & 1 deletion libraries/core-react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export * from './Avatar'
export * from './Search'
export { Slider } from './Slider'
export * from './Tooltip'
export { Snackbar } from './Snackbar'
export * from './Snackbar'
export * from './Popover'
export * from './Banner'
export * from './SelectionControls'
Expand Down