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: Video coverage extended #23797

Merged
merged 2 commits into from
Aug 10, 2023
Merged

Docs: Video coverage extended #23797

merged 2 commits into from
Aug 10, 2023

Conversation

jonniebigodes
Copy link
Contributor

@jonniebigodes jonniebigodes commented Aug 10, 2023

What I did

Introduced some additional videos to complement the documentation further. Featured in this pull request are the following pages:

  • Writing stories / AutoDocs (custom template section)
  • Writing Docs / Doc Blocks
  • Sharing / Embed
  • Sharing / Publish
  • API / Doc Blocks
    • ArgTypes
    • Canvas
    • Controls
    • Description
    • Primary
    • Source
    • Stories
    • Story
    • Subtitle
    • Title

How to test

  1. Check out the test_timestamps branch on the frontpage repository.
  2. Follow the steps in the contributing instructions for this branch, chore_docs_adds_videos.
  3. Open the relevant documentation, and the videos should be available, including the timestamped ones.

@chantastic and @kylegach when you have a moment, can you take a look at this and let me know of any feedback? Thanks in advance.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

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.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

💡 Internally, Storybook uses a similar implementation to generate the default template. See the Doc Blocks [API reference](./doc-blocks.md#available-blocks) to learn more about how Doc Blocks work.

</div>
<YouTubeCallout id="q8SY4yyNE6Q" title="Custom Autodocs with Storybook 7 Docs Page | Quick Tips" />
Copy link
Contributor

Choose a reason for hiding this comment

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

When we include these at the page level, we place them immediately under the page title. The thinking there being that videos offer an alternative learning path, and we want to present that as early as possible.

I think we should apply the same pattern here, and place the YouTubeCallout immediately after the section heading.

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 know, and I thought of it as well, but I'm aware that users might want to get into the video upfront, but when taking a second look at it, I placed it there to give users the code first, and after it the video and the text so that they follow along what's being presented. More than ok with moving it up and seeing how it fares. If need be we can shift it around later on.

Sounds good?

Copy link
Contributor

@kylegach kylegach 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 nice! I left a suggestion, but I'll go ahead and approve.

Copy link

@chantastic chantastic 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 taking this on João!
Cool to see that you were able to add the specific time stamps for the doc blocks!

@jonniebigodes
Copy link
Contributor Author

Thank you both for the speedy review. Appreciate it 🙏 ! I've just addressed the suggestion, and let's see how it fares. Merging this in a bit after the frontpage one is merged in.

@jonniebigodes jonniebigodes merged commit af906e2 into next Aug 10, 2023
15 checks passed
@jonniebigodes jonniebigodes deleted the chore_docs_adds_videos branch August 10, 2023 21:29
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