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

Feat/15951/add button secondary #16097

Merged
merged 6 commits into from
Oct 7, 2022
Merged

Conversation

garrettbear
Copy link
Contributor

@garrettbear garrettbear commented Oct 5, 2022

Explanation

Adding ButtonSecondary built off of ButtonBase.

(similar to ButtonPrimary)

Sizes: SM, MD, & LG
Types: Normal (default) and Danger (enabled with danger prop = true)

Figma

More Information

Screenshots/Screencaps

Screen Shot 2022-10-05 at 11 51 24 AM

Screen Shot 2022-10-05 at 12 02 27 PM

Manual Testing Steps

yarn test:unit:jest .ui/components/component-library/button-secondary/button-secondary.test.js

Pre-Merge Checklist

  • PR template is filled out
  • IF this PR fixes a bug, a test that would have caught the bug has been added
  • PR is linked to the appropriate GitHub issue
  • PR has been added to the appropriate release Milestone

+ If there are functional changes:

  • Manual testing complete & passed
  • "Extension QA Board" label has been applied

@garrettbear garrettbear added team-design-system All issues relating to design system in Extension IA/NAV labels Oct 5, 2022
@garrettbear garrettbear self-assigned this Oct 5, 2022
@garrettbear garrettbear requested a review from a team as a code owner October 5, 2022 19:07
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2022

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.

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.

Some tiny suggestions otherwise 🔥🔥🔥

@metamaskbot
Copy link
Collaborator

Builds ready [77424c7]
Page Load Metrics (2440 ± 105 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint962370229492236
domContentLoaded205128822424222107
load205129052440220105
domInteractive205128822424222107

highlights:

storybook

Copy link
Member

@NidhiKJha NidhiKJha left a comment

Choose a reason for hiding this comment

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

LGTM! 🔥🔥

Comment on lines 108 to 110
<>
<ButtonSecondary {...args} />
</>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Do we need a fragment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope. can be removed.

@metamaskbot
Copy link
Collaborator

Builds ready [f123e52]
Page Load Metrics (2404 ± 136 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint872081132612
domContentLoaded198130522387277133
load198130782404283136
domInteractive198130522387277133

highlights:

storybook

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 🔥

@metamaskbot
Copy link
Collaborator

Builds ready [1804d0c]
Page Load Metrics (2217 ± 64 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint872352215490235
domContentLoaded18962452219813666
load18962452221713364
domInteractive18962452219813666

highlights:

storybook

@garrettbear garrettbear merged commit 958cfe6 into develop Oct 7, 2022
@garrettbear garrettbear deleted the feat/15951/add-button-secondary branch October 7, 2022 15:50
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
team-design-system All issues relating to design system in Extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Ext Nav] Create component: ButtonSecondary
4 participants