♻️ Migrate stories 54..61 to args#37860
Conversation
| args: { | ||
| width: '640px', | ||
| height: '360px', | ||
| ariaLabel: 'Video Player', |
There was a problem hiding this comment.
nit: use aria-label so you don't have to extract it and rename it in the fixtures
There was a problem hiding this comment.
using a hyphen in an arg name gives me a linting error, i dont think i can get around extracting and renaming in this case.
There was a problem hiding this comment.
you can turn off the rule with an eslint-disable-next-line comment otherwise feel free to merge
| }; | ||
|
|
||
| Default.argTypes = { | ||
| amount: { |
There was a problem hiding this comment.
nit/fmi: why do you use args for some and argTypes for others? amount: 1 is equivalent to {name: 'amount', control: {type: 'number'}, defaultValue: 1,}, right?
There was a problem hiding this comment.
Yep thats correct. The difference is that storybook will infer a control type based on the initial value of the arg and for argTypes we specify the control type. In this case since these would all probably be inferred correctly they really could all be args.
kvchari
left a comment
There was a problem hiding this comment.
some (non-blocking) comments
inferred i.e. strings, numbers, and booleans.
|
Warning: disparity between this PR Percy build and its The Percy build for this PR was approved (either manually by a member of the AMP team, or automatically if there were no visual diffs). However, during a continuous integration step we generated another Percy build using the commit on the This is possibly an indication of an issue with this pull request, but could also be the result of flakiness. Please inspect the two builds < This PR's Percy build /
|
Partial for #35923.