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

Docs: Story globals improvements #28856

Merged
merged 11 commits into from
Aug 26, 2024
Merged

Docs: Story globals improvements #28856

merged 11 commits into from
Aug 26, 2024

Conversation

kylegach
Copy link
Contributor

@kylegach kylegach commented Aug 9, 2024

What I did

Further updates to #28716

  • Improve feature flag callouts
  • Add API details to migration notes and clarify examples
  • Restructure guides to use sub-heading structure instead of snippet tabs - Offers ability to explain differences between APIs more thoroughly - Also a better experience, because users' tab preference will not apply across snippets
  • Clarify example snippets - Only demonstate one concept per snippet
  • Adjust heading levels
  • Add globals API reference section
  • Improve snippet filenames - Remove redundant storybook- prefix
  • Prose tweaks

Checklist for Contributors

Testing

Manual testing

Please review in locally running docs site.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

Greptile Summary

This PR updates the documentation for the Backgrounds and Viewport addons in Storybook, introducing the new globals API and feature flags. Key changes include:

  • Added new parameters format for addon backgrounds using options property
  • Updated Viewport addon configuration to use options instead of viewports
  • Introduced backgroundsStoryGlobals and viewportStoryGlobals feature flags
  • Revised API sections for both addons, including new globals and parameters
  • Updated code snippets to demonstrate usage with and without the globals API
  • Restructured guides to use sub-heading structure for better explanation of API differences

- Improve feature flag callouts
- Add API details to migration notes and clarify examples
- Restructure guides to use sub-heading structure instead of snippet tabs
    - Offers ability to explain differences between APIs more thoroughly
    - Also a better experience, because users' tab preference will not apply across snippets
- Clarify example snippets
    - Only demonstate one concept per snippet
- Adjust heading levels
- Add globals API reference section
- Improve snippet filenames
    - Remove redundant `storybook-` prefix
- Prose tweaks
@kylegach kylegach self-assigned this Aug 9, 2024
MIGRATION.md Show resolved Hide resolved
Copy link

nx-cloud bot commented Aug 9, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f73a7df. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

- Explain globals API and feature flag once, at the top
- Reference that callout in consistent manner throughout page
- Wrap entire docs section in "feature flag callout" to better differentiate
- Remove headings for with/without feature flag sections
    - Headings will be removed in SB 9+, so they're not a stable reference
Copy link
Contributor

@jonniebigodes jonniebigodes left a comment

Choose a reason for hiding this comment

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

This is shaping up nicely. Left some items for you to look into when you can

docs/essentials/backgrounds.mdx Outdated Show resolved Hide resolved
docs/essentials/backgrounds.mdx Outdated Show resolved Hide resolved
docs/essentials/backgrounds.mdx Outdated Show resolved Hide resolved
docs/essentials/backgrounds.mdx Outdated Show resolved Hide resolved
- Polish and details
- Make Viewport match structure and style of Backgrounds
@kylegach kylegach added the ci:docs Run the CI jobs for documentation checks only. label Aug 15, 2024
Co-authored-by: jonniebigodes <[email protected]>
@kylegach kylegach marked this pull request as ready for review August 15, 2024 03:15
* next: (281 commits)
  fix
  add `node:` prefixes
  move nx cache dir up to avoid vite watching it for our main storybook
  Bump version from "8.3.0-alpha.5" to "8.3.0-alpha.6" [skip ci]
  Write changelog for 8.3.0-alpha.6 [skip ci]
  minor fixes
  Yeah... don't look at this
  simplify eslint config further
  more fixes
  cleanup `no-extraneous-dependencies`
  fix style
  change pool options in sandboxes
  revamp sandbox setup to move to a workspace file
  fix typescript issue
  skip vitest integration in nextjs sandbox
  skip sveltekit tests
  fix script
  increase test timeout
  fix parallelism and skip more stories
  fix windows unit tests
  ...
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

27 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings

docs/_snippets/addon-backgrounds-options-in-meta.md Outdated Show resolved Hide resolved
docs/_snippets/addon-backgrounds-values-in-preview.md Outdated Show resolved Hide resolved
* next: (27 commits)
  apply fix for #28883
  fixes
  apply linting to scripts
  update snapshots
  cleanup
  remove duplicate comment
  more fixes
  jsdoc linting fixes
  misc fixes
  remove invalid syntax
  remove duplicate comment
  remove duplicate comment
  remove duplicate comment
  remove duplicate comment
  remove duplicate comment
  move comment
  move comment
  remove redundant comment
  fix linting of jsdoc comments example blocks
  fix snapshots
  ...
Copy link
Contributor

@jonniebigodes jonniebigodes left a comment

Choose a reason for hiding this comment

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

Thanks for adding the updates. I left some items for you to look into when you can.

docs/essentials/backgrounds.mdx Outdated Show resolved Hide resolved
docs/essentials/backgrounds.mdx Outdated Show resolved Hide resolved
docs/essentials/viewport.mdx Outdated Show resolved Hide resolved
docs/essentials/viewport.mdx Outdated Show resolved Hide resolved
docs/_snippets/addon-backgrounds-values-in-meta.md Outdated Show resolved Hide resolved
docs/essentials/viewport.mdx Outdated Show resolved Hide resolved
docs/essentials/toolbars-and-globals.mdx Outdated Show resolved Hide resolved
Comment on lines 90 to 98
To ensure that a story always uses a specific global value, regardless of what has been chosen in the toolbar, you can set the `globals` annotation on a story or component. This overrides the global value for those stories and disables the toolbar menu for that global when viewing the stories.

{/* prettier-ignore-start */}

<CodeSnippets path="component-story-theme-override-globals.md" />
<CodeSnippets path="addon-backgrounds-define-globals.md" />

{/* prettier-ignore-end */}

When Storybook loads the stories, it applies the selected theme to every story except the `Dark` story, which automatically applies the `dark` theme. It also turns off the toolbar dropdown with the theme options, preventing them from being changed and notifying that this story requires a specific value to be used.
In the example above, Storybook will force all Button stories to use a gray background color, except the `OnDark` story, which will use the dark background. For all Buttons stories, the toolbar menu will be disabled for the `backgrounds` global, with a tooltip explaining that the global is set at the story level.
Copy link
Contributor

Choose a reason for hiding this comment

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

When you're able, can you adjust the first paragraph or the one after the example, as we're repeating the same information, specifically that the toolbar is disabled when the user sets the globals option with the only caveat that it shows a tooltip.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm OK with a little repetition here. The first paragraph explains the behavior in general, and the second explains it as it specifically relates to the code snippet. I think having both is helpful.

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 no issue with repetition, but my aim here would be to change the wording a bit so that we repeat the exact same information in both places

* next: (60 commits)
  Version it
  Make sure local @storybook/cli packages is used in linked mode
  add default options for addon-viewports via parameters
  Fix tests
  Fix default viewports
  Bump version from "8.3.0-alpha.7" to "8.3.0-alpha.8" [skip ci]
  Write changelog for 8.3.0-alpha.8 [skip ci]
  fix lint
  fix lint
  fix lint
  fix types
  Addon Vitest: Improve transformation logic to avoid duplicate tests
  fix lint
  fix lint
  fix formatting
  SvelteKit: Add portable stories vite plugin
  add vite plugin to sveltekit
  increase CI class for vitest integration
  Addon-docs: Remove babel dependency
  Bump version from "8.3.0-alpha.6" to "8.3.0-alpha.7" [skip ci]
  ...
@kylegach kylegach requested a review from jonniebigodes August 22, 2024 17:30
@jonniebigodes
Copy link
Contributor

I'm good with it, and I approved it to unblock you. I left one item for you to look into when you have a moment.

@kylegach kylegach merged commit 1ebcb33 into next Aug 26, 2024
9 checks passed
@kylegach kylegach deleted the update-story-globals-docs branch August 26, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:docs Run the CI jobs for documentation checks only. documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants