-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: Storybook story improvements #294
Conversation
travis
commented
Jan 26, 2023
•
edited
Loading
edited
- Add SpaceFinder to Storybook
- Add Authenticator to Storybook
- Break Tailwind classes in SpaceFinder, Authenticator and UploadsList down to their raw CSS so that the components work and look good in Storybook.
- Extract SpaceCreator to a new component
Break tailwind classes down to their raw CSS so that it works and looks good in Storybook.
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ae4a1ef:
|
tests suddenly started failing in GitHub, with logs like: ``` examples/test/playwright test: ╔═════════════════════════════════════════════════════════════════════════╗ examples/test/playwright test: ║ Looks like Playwright Test or Playwright was just installed or updated. ║ examples/test/playwright test: ║ Please run the following command to download new browsers: ║ examples/test/playwright test: ║ ║ examples/test/playwright test: ║ npx playwright install ║ examples/test/playwright test: ║ ║ examples/test/playwright test: ║ <3 Playwright Team ║ examples/test/playwright test: ╚═════════════════════════════════════════════════════════════════════════╝ ``` running the tests locally also had this problem, and it went away once I ran `npx install playwright` add `playwright install` before all test runs to ensure this always happens
my last commit broke the w3console space finder, fix that!
this gets the uploads list looking better in storybook
we only have a "dark" theme for now, so default to a dark background in the stories
port CSS from Tailwind to raw CSS, rename some classes, factor common CSS out to base.css
get it into Storybook, extract css classes
@@ -4,7 +4,7 @@ | |||
"type": "module", | |||
"scripts": { | |||
"serve": "serve ../../", | |||
"test": "playwright test", | |||
"test": "playwright install && playwright test", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be reverted now #298 landed
@@ -0,0 +1,20 @@ | |||
import { AuthenticationSubmitted } from '@w3ui/react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
musing: perhaps we should make the naming consistent here so that we have ComponentName
and ComponentNameStateName
import { AuthenticationSubmitted } from '@w3ui/react' | |
import { AuthenticatorSubmitted } from '@w3ui/react' |
like
SpaceCreator
->SpaceCreatorCreating
or perhaps we should export a "view(s) only" component, that takes a state prop that we can set from a single story per Component, so our docs pages show all the things... tho that would force us to make a component that takes all the needed state and handlers for each of it's states, so perhaps the current pattern is better for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeaaaaa - I'm not totally happy with the naming/structure here either, but similarly uncertain on the best "API design" here - I'll leave it as it is, but am doing a pass getting ready for Adam to come in and do some design work and will keep this in mind as I do, thanks!
}, | ||
parameters: { | ||
backgrounds: { | ||
default: 'dark' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is neat!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh let's set it in the defaults rather than per story for now
https://storybook.js.org/docs/react/essentials/backgrounds#configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while we're noodling with defaults, no one wants to see times new roman lets set at least a nice default font via
https://storybook.js.org/docs/react/configure/story-rendering#adding-to-body
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so real. will do!
packages/react/src/SpaceFinder.tsx
Outdated
@@ -24,18 +24,18 @@ export function SpaceFinder ({ spaces, selected, setSelected, className }: Space | |||
) | |||
|
|||
return ( | |||
<div className={className}> | |||
<div className={`${className} w3ui-space-finder-wrapper`}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe use the no suffix class name for the root element like
<div className={`${className} w3ui-space-finder-wrapper`}> | |
<div className={`${className} w3ui-space-finder`}> |
...cos looks cooler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which is not normally a relevant metric, but if we want devs to feel nice when they customise our thingers it becomes more important!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed! I think there was a case in another component where I felt like -wrapper
made sense, but this ain't it
@@ -7,7 +7,7 @@ function Uploads ({ uploads }: { uploads?: UploadListResult[] }): JSX.Element { | |||
return ( | |||
<> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd be down for always returning a root node with the w3ui-kebab-cased component name as the classname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh - we actually do this already in the UploadsList
function at the bottom of this file - this function is private and called from that one, mainly to alias props.uploadsList?.[0].data
to uploads
without breaking the render-props style here
import '@w3ui/react/src/styles/authenticator.css' | ||
|
||
export default { | ||
title: 'w3ui/AuthenticationSubmitted', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe force this to appear in the same section, and change the export name like
title: 'w3ui/AuthenticationSubmitted', | |
title: 'w3ui/Authenticator', // force name to match so it appears in same section |
then later in the file export it with a different name to distinguish it from the other one
export const Submitted = {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SICK, great idea, this is much nicer
export default { | ||
title: 'w3ui/SpaceCreatorCreating', | ||
component: SpaceCreatorCreating, | ||
tags: ['autodocs'], | ||
argTypes: { | ||
}, | ||
parameters: { | ||
backgrounds: { | ||
default: 'dark' | ||
} | ||
} | ||
} | ||
|
||
export const Primary = { | ||
args: { | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, we can fix up the titles and exports to group states of the same logical components
export default { | |
title: 'w3ui/SpaceCreatorCreating', | |
component: SpaceCreatorCreating, | |
tags: ['autodocs'], | |
argTypes: { | |
}, | |
parameters: { | |
backgrounds: { | |
default: 'dark' | |
} | |
} | |
} | |
export const Primary = { | |
args: { | |
} | |
} | |
export default { | |
title: 'w3ui/SpaceCreator', | |
component: SpaceCreatorCreating, | |
tags: ['autodocs'], | |
argTypes: { | |
}, | |
parameters: { | |
backgrounds: { | |
default: 'dark' | |
} | |
} | |
} | |
export const Creating = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
margin-bottom: 1rem; | ||
} | ||
|
||
.w3-uploads-list nav>*+*{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have never seen one like that before!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right? cribbed from the tailwind docs! eg, https://tailwindcss.com/docs/space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love it. Some naming suggestions and tweaks suggested inline
🤖 I have created a release *beep* *boop* --- ## 1.0.0 (2023-03-23) ### ⚠ BREAKING CHANGES * use new account model ([#400](#400)) ### Features * add terms of service page ([#417](#417)) ([6deb24d](6deb24d)) * adds space-finder autocomplete combobox ([#268](#268)) ([3dcd647](3dcd647)) * allow users to set page size in W3APIProvider ([#308](#308)) ([814a293](814a293)) * club tropical w3 auth boxen ([#350](#350)) ([2266eb2](2266eb2)) * delegate access to spaces ([#293](#293)) ([441d757](441d757)) * import a space into w3console ([#309](#309)) ([a69a95b](a69a95b)) * Improve upload component flow ([#285](#285)) ([ba9a3bf](ba9a3bf)) * publish console to IPFS & Cloudflare from CI ([#287](#287)) ([e2a833e](e2a833e)) * Storybook story improvements ([#294](#294)) ([e0de2cc](e0de2cc)) * use new account model ([#400](#400)) ([66dd20b](66dd20b)) * w3console example app ([#255](#255)) ([df08029](df08029)) ### Bug Fixes * add _headers ([#418](#418)) ([4eb1da1](4eb1da1)) * fix w3console styling ([#320](#320)) ([74a298c](74a298c)) * remove authenticator class when registed ([#352](#352)) ([3668f3b](3668f3b)) * w3console polish ([#284](#284)) ([9a67365](9a67365)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## 1.0.0 (2023-11-28) ### Features * adds space-finder autocomplete combobox ([#268](#268)) ([3dcd647](3dcd647)) * allow users to set page size in W3APIProvider ([#308](#308)) ([814a293](814a293)) * club tropical w3 auth boxen ([#350](#350)) ([2266eb2](2266eb2)) * Customizable UI components ([#208](#208)) ([0a776fe](0a776fe)) * implement reverse paging ([#381](#381)) ([10f059a](10f059a)) * Improve upload component flow ([#285](#285)) ([ba9a3bf](ba9a3bf)) * simplify ([#591](#591)) ([d1dfdf0](d1dfdf0)) * Storybook story improvements ([#294](#294)) ([e0de2cc](e0de2cc)) ### Bug Fixes * fix w3console styling ([#320](#320)) ([74a298c](74a298c)) * remove authenticator class when registed ([#352](#352)) ([3668f3b](3668f3b)) * w3console polish ([#284](#284)) ([9a67365](9a67365)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Alan Shaw <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [1.2.0](react-v1.1.1...react-v1.2.0) (2023-11-30) ### Features * add a logout function ([#595](#595)) ([0995fd5](0995fd5)) * adds space-finder autocomplete combobox ([#268](#268)) ([3dcd647](3dcd647)) * allow users to set page size in W3APIProvider ([#308](#308)) ([814a293](814a293)) * club tropical w3 auth boxen ([#350](#350)) ([2266eb2](2266eb2)) * Customizable UI components ([#208](#208)) ([0a776fe](0a776fe)) * implement reverse paging ([#381](#381)) ([10f059a](10f059a)) * Improve upload component flow ([#285](#285)) ([ba9a3bf](ba9a3bf)) * simplify ([#591](#591)) ([d1dfdf0](d1dfdf0)) * Storybook story improvements ([#294](#294)) ([e0de2cc](e0de2cc)) ### Bug Fixes * fix w3console styling ([#320](#320)) ([74a298c](74a298c)) * homepage URL in package.json ([1229119](1229119)) * remove authenticator class when registed ([#352](#352)) ([3668f3b](3668f3b)) * w3console polish ([#284](#284)) ([9a67365](9a67365)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>