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

feat: Storybook story improvements #294

Merged
merged 13 commits into from
Feb 1, 2023
Merged

feat: Storybook story improvements #294

merged 13 commits into from
Feb 1, 2023

Conversation

travis
Copy link
Member

@travis travis commented Jan 26, 2023

  1. Add SpaceFinder to Storybook

SpaceFinder

  1. Add Authenticator to Storybook

Authenticator

  1. Break Tailwind classes in SpaceFinder, Authenticator and UploadsList down to their raw CSS so that the components work and look good in Storybook.

UploadsList

  1. Extract SpaceCreator to a new component

SpaceCreator

Break tailwind classes down to their raw CSS so that it works and looks good in Storybook.
@travis travis marked this pull request as ready for review January 26, 2023 05:03
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 26, 2023

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:

Sandbox Source
@w3ui/example-react-file-upload Configuration
@w3ui/example-react-sign-up-in Configuration
@w3ui/example-react-uploads-list Configuration
@w3ui/example-solid-file-upload Configuration
@w3ui/example-solid-sign-up-in Configuration
@w3ui/example-solid-uploads-list Configuration
@w3ui/example-vue-file-upload Configuration
@w3ui/example-vue-sign-up-in Configuration
@w3ui/example-vue-uploads-list Configuration

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!
@github-actions
Copy link
Contributor

github-actions bot commented Jan 26, 2023

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
@travis travis changed the title feat: add SpaceFinder to StoryBook feat: StoryBook story improvements Jan 26, 2023
@travis travis requested a review from olizilla January 26, 2023 22:05
port CSS from Tailwind to raw CSS, rename some classes, factor common CSS out to base.css
@travis travis requested a review from alanshaw January 26, 2023 22:34
@travis travis changed the title feat: StoryBook story improvements feat: Storybook story improvements Jan 26, 2023
@travis travis linked an issue Jan 27, 2023 that may be closed by this pull request
@@ -4,7 +4,7 @@
"type": "module",
"scripts": {
"serve": "serve ../../",
"test": "playwright test",
"test": "playwright install && playwright test",
Copy link
Contributor

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'
Copy link
Contributor

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

Suggested change
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.

Copy link
Member Author

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'
Copy link
Contributor

Choose a reason for hiding this comment

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

this is neat!

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Member Author

Choose a reason for hiding this comment

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

so real. will do!

@@ -24,18 +24,18 @@ export function SpaceFinder ({ spaces, selected, setSelected, className }: Space
)

return (
<div className={className}>
<div className={`${className} w3ui-space-finder-wrapper`}>
Copy link
Contributor

@olizilla olizilla Jan 31, 2023

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

Suggested change
<div className={`${className} w3ui-space-finder-wrapper`}>
<div className={`${className} w3ui-space-finder`}>

...cos looks cooler.

Copy link
Contributor

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!

Copy link
Member Author

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 (
<>
Copy link
Contributor

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

Copy link
Member Author

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',
Copy link
Contributor

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

Suggested change
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 = {

Copy link
Member Author

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

Comment on lines 4 to 20
export default {
title: 'w3ui/SpaceCreatorCreating',
component: SpaceCreatorCreating,
tags: ['autodocs'],
argTypes: {
},
parameters: {
backgrounds: {
default: 'dark'
}
}
}

export const Primary = {
args: {
}
}
Copy link
Contributor

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

Suggested change
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 = {}

Copy link
Member Author

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>*+*{
Copy link
Contributor

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!

Copy link
Member Author

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

Copy link
Contributor

@olizilla olizilla left a 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

@travis travis merged commit e0de2cc into main Feb 1, 2023
@travis travis deleted the storybook-components branch February 1, 2023 02:38
travis pushed a commit that referenced this pull request Mar 23, 2023
🤖 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>
alanshaw pushed a commit that referenced this pull request Nov 28, 2023
🤖 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]>
travis pushed a commit that referenced this pull request Nov 30, 2023
🤖 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

space manager customizable component
2 participants