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

Chore: (Docs) Writing stories section #16236

Merged
merged 20 commits into from
Oct 20, 2021

Conversation

jonniebigodes
Copy link
Contributor

With this pull request the writing stories is updated in light of the changes required for 6.4, follows up on #15519.

What was done:

  • TOC was updated to include the play function documentation.
  • The entire section was polished a bit for clarity.
  • Added the play function documentation.
  • Created snippets for said documentation (based on currently documented frameworks and MDX)
  • Updated the section's mdx snippets in light of 6.4 and render functions.

One item of feedback. The play function's snippets are placeholders and will require adjustments per the addon-interactions release. @ghengeveld I'll follow up with you on this.

@kylegach if you'd be so kind to take a look and let me know of any feedback you might encounter.

Feel free to provide feedback.

@nx-cloud
Copy link

nx-cloud bot commented Oct 5, 2021

Nx Cloud Report

CI ran the following commands for commit e3bf61c. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.


When Storybook loads the story, it interacts with the component, filling in its inputs and triggering any validation logic defined.

## Working with events
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this section and the prior one should flip in order, as this one (sort of) serves as an introduction to userEvent & fireEvent, but the prior one uses userEvent.

Copy link
Contributor

@kylegach kylegach Oct 5, 2021

Choose a reason for hiding this comment

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

Actually, I had a different thought after shifting from reviewing the words to the snippets...

I think we should combine these two sections, and point to how you can use testing-library packages to query for elements. This has a couple benefits:

  1. It avoids the order issue I commented on
  2. It allows us to jump straight into the most appropriate method of querying for elements
    • Querying by data-testid is a last resort, according to testing-library best practices, and we should not be encouraging it in our snippets, imo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kylegach With the latest commit, I've condensed the two sections and expanded the examples a bit. Let me know

Copy link
Contributor

Choose a reason for hiding this comment

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

Much better! Nice job reworking them into a a better narrative flow. I left a few more comments.

@jonniebigodes
Copy link
Contributor Author

Self merging this

@jonniebigodes jonniebigodes merged commit cef029e into 6-4-docs Oct 20, 2021
@jonniebigodes jonniebigodes deleted the chore_polish_writing_stories branch October 20, 2021 13:41
@shilman shilman added this to the 6.4 PRs milestone Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants