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.

Sign up for GitHub

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

Jump to bottom

Docs: Svelte CSF snippets #29546

Merged
merged 15 commits into from
Nov 15, 2024
Merged

Docs: Svelte CSF snippets #29546

merged 15 commits into from
Nov 15, 2024

Conversation

jonniebigodes
Copy link
Contributor

@jonniebigodes jonniebigodes commented Nov 5, 2024 •
edited by greptile-apps bot
Loading

With this pull request, the documentation was updated to contain the first batch of Svelte CSF snippets.

What was done:

  • Removed the old native-format snippets
  • Added updated Svelte CSF snippets
  • Fixed snippets
  • Polished documentation to align with the upcoming release of the addon, including changes to the framework pages

Checklist for Contributors

Manual testing

  1. In this repo, check out this branch, docs_adjust_svelte_snippets_v5
  2. In the web repo
    1. Run npm run sync-docs
    2. In another terminal tab, run npm run dev
    3. Open the docs page http://localhost:3000/docs/

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.

@JReinhold, when you have a moment, can you fact-check this and provide some guidance on some of the items? They are tagged accordingly with comments. Thanks in advance.

Greptile Summary

This PR updates Storybook's documentation to modernize Svelte examples by replacing old native-format snippets with new Component Story Format (CSF) examples across multiple documentation files.

  • Removed outdated native-format Svelte snippets from all documentation files
  • Added new Svelte CSF examples using @storybook/addon-svelte-csf package and defineMeta function
  • Added tabbed navigation between CSF and Svelte CSF formats for better organization
  • Standardized prop naming (e.g., 'backgroundColor' instead of 'background') across framework examples
  • Added TODO comments marked with 'TK' for examples that need further review against recommendations

@jonniebigodes jonniebigodes self-assigned this Nov 5, 2024
@jonniebigodes jonniebigodes added documentation ci:docs Run the CI jobs for documentation checks only. labels Nov 5, 2024
@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Nov 5, 2024 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f922ed1. 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.

greptile-apps[bot]
greptile-apps bot reviewed Nov 5, 2024
View reviewed changes
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.

30 file(s) reviewed, 50 comment(s)
Edit PR Review Bot Settings | Greptile

docs/_snippets/button-group-story.md Outdated Show resolved Hide resolved
docs/_snippets/button-group-story.md Show resolved Hide resolved
docs/_snippets/button-story-component-args-primary.md Show resolved Hide resolved
docs/_snippets/button-story-component-args-primary.md Show resolved Hide resolved
docs/_snippets/button-story-component-decorator.md Outdated Show resolved Hide resolved
docs/_snippets/component-story-static-asset-with-import.md Outdated Show resolved Hide resolved
docs/_snippets/component-story-static-asset-without-import.md Outdated Show resolved Hide resolved
docs/_snippets/component-story-with-accessibility.md Outdated Show resolved Hide resolved
docs/_snippets/component-story-with-accessibility.md Outdated Show resolved Hide resolved
docs/_snippets/component-story-with-accessibility.md Outdated Show resolved Hide resolved
@jonniebigodes jonniebigodes added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Nov 12, 2024
JReinhold
JReinhold reviewed Nov 12, 2024
View reviewed changes
docs/_snippets/page-story.md
```js filename="Page.stories.js" renderer="svelte" language="js"
```svelte filename="Page.stories.svelte" renderer="svelte" language="js" tabTitle="Svelte CSF"
<!-- TK: vet this against recommendation and apply it to the other languages (eg., ts-4-9, ts) -->
```
Copy link
Contributor

Choose a reason for hiding this comment

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

note that TypeScript will complain here, because it doesn't know that this is doable, but it works fine at runtime.

JReinhold
JReinhold reviewed Nov 12, 2024
View reviewed changes
docs/get-started/frameworks/svelte-vite.mdx
@@ -48,7 +48,9 @@ Storybook for Svelte & Vite is a [framework](../../contribute/framework.mdx) tha

#### Automatic migration

When running the `upgrade` command above, you should get a prompt asking you to migrate to `@storybook/sveltekit`, which should handle everything for you. In case that auto-migration does not work for your project, refer to the manual migration below.
{/* TK: Vet this for accuracy, the migration never prompted me to opt-in to the @storybook/sveltekit package, instead it updated the package version. Additionally the example refers to svelte-vite, not sveltekit */}
Copy link
Contributor

Choose a reason for hiding this comment

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

skipping for now

kylegach
kylegach reviewed Nov 12, 2024
View reviewed changes
docs/get-started/install.mdx
Comment on lines +190 to 196
Update your Storybook configuration file (i.e., `.storybook/main.js|ts`) to enable support for this format.

{/* prettier-ignore-start */}

<CodeSnippets path="main-config-csf-addon-register.md" />
<CodeSnippets path="main-config-svelte-csf-register.md" />

{/* prettier-ignore-end */}
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 redundant with what is available at /addons/install-addons.mdx#manual-installation.

I do not think we should set a precedent for including this every time we document the add command, as that will balloon maintenance efforts.

JReinhold
JReinhold approved these changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

Everything is good on my side.

Massive work @jonniebigodes 👏👏👏

@jonniebigodes
Copy link
Contributor Author

@JReinhold, thank you very much for the assistance and help on this pull request 🙏 !

@jonniebigodes jonniebigodes merged commit c7569a6 into next Nov 15, 2024
9 checks passed
@jonniebigodes jonniebigodes deleted the docs_adjust_svelte_snippets_v5 branch November 15, 2024 11:58
@github-actions github-actions bot mentioned this pull request Nov 15, 2024
Merged
11 tasks
@jonniebigodes jonniebigodes mentioned this pull request Nov 15, 2024
Merged
This was referenced Nov 16, 2024
Closed
Merged
yannbf pushed a commit that referenced this pull request Nov 20, 2024
@jonniebigodes @storybook-bot
Merge pull request #29546 from storybookjs/docs_adjust_svelte_snippet…
f840774 
…s_v5

Docs: Svelte CSF snippets
(cherry picked from commit c7569a6)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@shilman shilman shilman left review comments

@kylegach kylegach kylegach left review comments

@JReinhold JReinhold JReinhold approved these changes

Assignees

@jonniebigodes jonniebigodes

Labels
ci:docs Run the CI jobs for documentation checks only. documentation patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jonniebigodes @kylegach @shilman @JReinhold