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

Update knobs to control: MetamaskTemplateRenderer #18677

Merged
merged 3 commits into from
Apr 26, 2023

Conversation

dj-pundir
Copy link
Contributor

Explanation

The knobs are deprecated so its in continuity with other tasks to change the knobs with controls. Following are changes to the component metamask-template-renderer.stories.js

  • Change knob to control
  • Remove WithInvalidElement story altogether

Fixes: #18498

Screenshots/Screencaps

Here is

Before

image

After

image

Manual Testing Steps

yarn storybook

  • Go to Component -> App
  • Select MetamaskTemplateRenderer

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

@dj-pundir dj-pundir requested a review from a team as a code owner April 20, 2023 07:37
@dj-pundir dj-pundir requested a review from brad-decker April 20, 2023 07:37
@github-actions
Copy link
Contributor

github-actions bot commented Apr 20, 2023

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@dj-pundir
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

Looking good looks like there is a linting issue that needs to be fixed

Comment on lines -94 to -109
export const WithInvalidElement = () => (
<MetaMaskTemplateRenderer
sections={object('sections', [
{
...SECTIONS,
key: 'safe-tree',
},
{
element: 'Unsafe',
key: 'unsafe-tree',
children:
'I should be displayed, but I wont be due to unsafe component',
},
])}
/>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggested to @dj-pundir to remove this story as it was display an error intentionally but I don't think it adds much value in storybook

@dj-pundir
Copy link
Contributor Author

Looking good looks like there is a linting issue that needs to be fixed

Done the linting. Thanks!!

Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

LGTM! 💯 Thanks for your contribution!

@georgewrmarshall georgewrmarshall added the team-design-system All issues relating to design system in Extension label Apr 26, 2023
@brad-decker brad-decker merged commit d428a48 into MetaMask:develop Apr 26, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
external-contributor team-design-system All issues relating to design system in Extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update knobs to controls in storybook: MetamaskTemplateRenderer
3 participants