Skip to content

♻️ Migrate Stories 45..49 to args#37850

Merged
rbeckthomas merged 15 commits intoampproject:mainfrom
rbeckthomas:remove-knobs
Mar 15, 2022
Merged

♻️ Migrate Stories 45..49 to args#37850
rbeckthomas merged 15 commits intoampproject:mainfrom
rbeckthomas:remove-knobs

Conversation

@rbeckthomas
Copy link
Copy Markdown
Contributor

Partial for #35923.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Mar 10, 2022

This pull request introduces 1 alert when merging 1760d9f into b70a0f3 - view on LGTM.com

new alerts:

  • 1 for Variable not declared before use

@rbeckthomas rbeckthomas requested review from dmanek and kvchari March 10, 2022 23:27
Copy link
Copy Markdown
Contributor

@kvchari kvchari left a comment

Choose a reason for hiding this comment

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

nit: argTypes where the key matches the name property in the value can just be in args (except types that don't get automatically inferred correctly by their defaultValue like type: 'color').

also args should have names that match the name of the component's property/attribute binding, so we don't have to extract and rename the variable (e.g should use data-trackid instead of trackId).

@rbeckthomas rbeckthomas enabled auto-merge (squash) March 15, 2022 02:08
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