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

Disable play functions in docs mode, unless you set parameters.docs.autoplay #19659

Merged
merged 10 commits into from
Nov 4, 2022

Conversation

tmeasday
Copy link
Member

Issue: Play functions cause havoc on docs pages ;)

What I did

  • Don't play unless runPlayFunction is set
  • Pass that true by default
  • Pass that false from the <Story> doc block, unless parameters.docs.autoplay is set

How to test

See autoplay stories, or e2e/unit tests

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Nice one!!! 🚀

code/lib/preview-web/src/render/StoryRender.ts Outdated Show resolved Hide resolved
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.

In general I think this is good, but I don't understand why autoplay and runPlayFunction are different. As far as I understand they do the exact same thing, they're just applied in different places, so I would think it was simpler if they were both called autoplay.

Co-authored-by: Michael Shilman <shilman@users.noreply.github.com>
@tmeasday
Copy link
Member Author

I guess that's a fair point. I guess my thought process was that a pithy name makes more sense for a user facing parameter (autoplay) but a longer, more descriptive name made more sense for an internal API.

However, I agree, the confusion of having different names for the same thing is not worth the dubious benefit of the above.

@tmeasday
Copy link
Member Author

So the E2E test is failing for svelte SBs for a reason I think unrelated to this PR: the "primary" story in docs page isn't rendering properly:
image

Note, oddly, that the rest of the stories do render fine in docspage.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

I agree with @JReinhold 's comment on renaming runPlayFunction to autoplay. Otherwise LGTM!

@tmeasday
Copy link
Member Author

tmeasday commented Nov 1, 2022

Changes are made, this PR is just blocked on this now.

@tmeasday tmeasday assigned JReinhold and unassigned tmeasday Nov 1, 2022
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.

Looks good to me. I'll investigate the Svelte issue. It's been there for a while, I guess it's just now that we're catching it in our tests.

@tmeasday tmeasday merged commit b0b6c6b into next Nov 4, 2022
@tmeasday tmeasday deleted the tom/sb-888-disable-play-functions-in-docs-if-not branch November 4, 2022 01:46
@tmeasday
Copy link
Member Author

tmeasday commented Nov 4, 2022

Thanks for the assist @JReinhold !!

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.

None yet

3 participants