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

InputControl: Make onChange observable in Storybook #60055

Merged
merged 2 commits into from Mar 21, 2024
Merged

Conversation

mirka
Copy link
Member

@mirka mirka commented Mar 20, 2024

Required for testing IME keydown handling (follow-up to #59081)

What?

Enhances the Storybook stories for InputControl so onChange events can be tested using the Actions panel.

Why?

We generally want all our onChange events to be testable in Storybook.

Testing Instructions

In trunk Storybook, we can see that change events aren't logged in the Actions panel.

  1. In local Storybook for this PR, the change events should now be logged as expected.
  2. Enable the control for the isPressEnterToChange prop and reload.
  3. The onChange should only be logged after blurring the input or hitting the Enter key. The onValidate calls should also be logged.

@mirka mirka added [Type] Developer Documentation Documentation for developers [Package] Components /packages/components Storybook Storybook and its stories for components labels Mar 20, 2024
@mirka mirka requested a review from a team March 20, 2024 20:31
@mirka mirka self-assigned this Mar 20, 2024
@mirka mirka requested a review from ajitbohra as a code owner March 20, 2024 20:31
Copy link

github-actions bot commented Mar 20, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Does the job 👍

Any concerns with this bit from Storybook docs?

This is quite useful when your component has dozens (or hundreds) of methods and you do not want to manually apply the fn utility for each of those methods. However, this is not the recommended way of writing actions. That's because automatically inferred args are not available as spies in your play function. If you use argTypesRegex and your stories have play functions, you will need to also define args with the fn utility to test them in your play function.

I personally don't have concerns about tracking them all, was just curious if you had any thoughts about it.

Note: unit test failures are unrelated and should disappear after a rebase.

@mirka
Copy link
Member Author

mirka commented Mar 21, 2024

This information is new to me, thanks for surfacing! I don't foresee us using Play functions in this main Storybook instance, so I think the convenience of the catch-all still outweighs the potential downsides. In this PR too, I likely would've forgotten onValidate if it weren't for the catch-all. But I have no issue with going back to explicit declarations if we start to encounter problems down the line 👍

@mirka mirka merged commit e04cf9c into trunk Mar 21, 2024
56 checks passed
@mirka mirka deleted the input-control-story branch March 21, 2024 12:21
@github-actions github-actions bot added this to the Gutenberg 18.0 milestone Mar 21, 2024
carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Mar 27, 2024
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components Storybook Storybook and its stories for components [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants