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

Change default Storybook layout and improve Story sorting #2807

Merged
merged 4 commits into from Nov 18, 2021

Conversation

jpveooys
Copy link
Contributor

@jpveooys jpveooys commented Nov 17, 2021

Overview

This makes a couple of changes:

  • changes the default layout of stories from fullscreen to the default layout (padded) (while changing certain stories to use fullscreen where it makes sense)
  • sorts folders in Storybook alphabetically
  • changes the Meta and Story types to be imported from @storybook/react instead of @storybook/react/types-6-0

Link to preview

https://5e25c277526d380020b5e418-kqmsqxgvha.chromatic.com/

Reason

Layout

For many stories, a full-screen layout was problematic because it caused shadows and borders to be out of view on the Canvas tab:

image

With padding these can be properly seen:

image

I've gone through and manually applied the fullscreen layout to stories for components where it makes sense or wasn't getting in the way of borders or shadows.

(There shouldn't be any change to the Docs tab.)

Sorting

The change to sorting is to stop some story folders being sorted last:

image

Instead these are sorted alphabetically:

image

There is no change to the sorting of individual stories, but note that we are currently getting hit by this in production: storybookjs/storybook#15574

This is currently causing some individual stories to be in the wrong order in production e.g. https://storybook.design-system.digital.mod.uk/?path=/docs/range-slider--custom-value-formatter

Imports

This change is to simply make the imports in line with examples in the Storybook docs:

image

Work carried out

  • Remove the default fullscreen layout override
  • Go through stories and set certain ones to use a fullscreen layout as appropriate
  • Configure story sorting
  • Correct type imports

@jpveooys jpveooys added Package: react-component-library Package/code type Type: Development Related to a design system component labels Nov 17, 2021
@jpveooys jpveooys self-assigned this Nov 17, 2021
@jpveooys jpveooys marked this pull request as ready for review November 17, 2021 14:15
Copy link
Member

@m7kvqbe1 m7kvqbe1 left a comment

Choose a reason for hiding this comment

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

LGTM - just noticed some naming inconsistencies that could be fixed quickly in this PR.

The `fullscreen` layout is awkward for things like form controls, as shadows (and sometimes borders) are out of view.

This changes the default layout from `fullscreen` to the default `padded` layout, and goes through and applies the `fullscreen` layout to components where it makes sense or wasn't getting in the way of borders or shadows.
This makes sure story folders are sorted alphabetically, while keeping individual stories sorted by export order.
This changes the import of the Story and Meta types to be as in the examples in the Storybook docs.
@sonarcloud
Copy link

sonarcloud bot commented Nov 18, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jpveooys jpveooys merged commit 774725f into master Nov 18, 2021
@jpveooys jpveooys deleted the docs/storybook-layout branch November 18, 2021 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: react-component-library Package/code type Type: Development Related to a design system component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants