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

Added Tag Url Component #16159

Merged
merged 25 commits into from
Nov 10, 2022
Merged

Added Tag Url Component #16159

merged 25 commits into from
Nov 10, 2022

Conversation

NidhiKJha
Copy link
Member

@NidhiKJha NidhiKJha commented Oct 11, 2022

After

Without CTA

Screenshot 2022-10-13 at 10 35 09 PM

With CTA

Screenshot 2022-10-13 at 10 36 16 PM

Overflow Text

Screenshot 2022-10-13 at 10 37 49 PM

Manual Testing Steps

Storybook

  • Go to the latest build of the storybook in this PR
  • Search for TagUrl in component-library
  • Check the stories, controls, and documentation

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

@NidhiKJha NidhiKJha requested a review from a team as a code owner October 11, 2022 09:45
@NidhiKJha NidhiKJha added team-design-system All issues relating to design system in Extension DO-NOT-MERGE Pull requests that should not be merged IA/NAV labels Oct 11, 2022
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.

Great start, realize it's a draft but left a comment regarding the structure

ui/components/component-library/tag-url/tag-url.js Outdated Show resolved Hide resolved
@NidhiKJha NidhiKJha marked this pull request as draft October 11, 2022 19:37
@NidhiKJha NidhiKJha changed the title (WIP 👷‍♀️) Added Tag Url Component Added Tag Url Component Oct 13, 2022
@NidhiKJha NidhiKJha marked this pull request as ready for review October 13, 2022 17:09
@NidhiKJha NidhiKJha requested a review from garrettbear October 13, 2022 17:10
@metamaskbot
Copy link
Collaborator

Builds ready [d432db9]
Page Load Metrics (2350 ± 58 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint892961144320
domContentLoaded21832582232911455
load21832603235012158
domInteractive21832582232911455

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [0c7b309]
Page Load Metrics (2241 ± 114 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint871941162613
domContentLoaded169728052225233112
load169728052241237114
domInteractive169728052225233112

highlights:

storybook

@NidhiKJha NidhiKJha removed the DO-NOT-MERGE Pull requests that should not be merged label Oct 14, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [2b437fe]
Page Load Metrics (2382 ± 79 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint852258220468225
domContentLoaded21362710236716579
load21392710238216579
domInteractive21362710236716579

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.

Looking good. I was playing around with removing some styles in favour of box props. It should make it more responsive

Heres the branch https://github.com/MetaMask/metamask-extension/tree/fix-15104-suggestions

suggestions.mov

ui/components/component-library/tag-url/tag-url.scss Outdated Show resolved Hide resolved
ui/components/component-library/tag-url/README.mdx Outdated Show resolved Hide resolved
ui/components/component-library/tag-url/README.mdx Outdated Show resolved Hide resolved
ui/components/component-library/tag-url/README.mdx Outdated Show resolved Hide resolved
ui/components/component-library/tag-url/README.mdx Outdated Show resolved Hide resolved
@NidhiKJha NidhiKJha requested a review from darkwing October 25, 2022 18:47
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 really good! A few tiny things that I've changed my mind about apologies! It maybe in future that we need to extend this component so we can change they type of action button but we can cross that bridge when we get to it.

ui/components/component-library/tag-url/README.mdx Outdated Show resolved Hide resolved
ui/components/component-library/tag-url/tag-url.js Outdated Show resolved Hide resolved
ui/components/component-library/tag-url/tag-url.js Outdated Show resolved Hide resolved
ui/components/component-library/tag-url/tag-url.stories.js Outdated Show resolved Hide resolved
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. Just need to get build to pass

@metamaskbot
Copy link
Collaborator

Builds ready [0520209]
Page Load Metrics (2428 ± 142 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint97163123188
domContentLoaded192629572407300144
load192629572428297142
domInteractive192629572407300144

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [dbd945d]
Page Load Metrics (2224 ± 109 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint97145114147
domContentLoaded181425682197223107
load183326692224226109
domInteractive181425682197223107

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.

Looking great! Was just going over the house keeping criteria. Could we add some stories for label and src?

@metamaskbot
Copy link
Collaborator

Builds ready [1342296]
Page Load Metrics (2393 ± 152 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1052503252517248
domContentLoaded176027702362316152
load177829322393317152
domInteractive176027702362316152

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!

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.

one nit but not blocking

ui/components/component-library/tag-url/tag-url.stories.js Outdated Show resolved Hide resolved
@metamaskbot
Copy link
Collaborator

Builds ready [4a51501]
Page Load Metrics (2560 ± 303 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint961068215280134
domContentLoaded186142112537630303
load186242112560631303
domInteractive186142112537630303
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 5906 bytes
  • ui: 12717 bytes
  • common: 4237 bytes

highlights:

storybook

@github-actions
Copy link
Contributor

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.

@metamaskbot
Copy link
Collaborator

Builds ready [e956d46]
Page Load Metrics (2654 ± 209 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint10287318617082
domContentLoaded215040122639440211
load215040122654435209
domInteractive215040122639440211
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 5914 bytes
  • ui: 18055 bytes
  • common: 4237 bytes

highlights:

storybook

@NidhiKJha NidhiKJha merged commit 4a247a9 into develop Nov 10, 2022
@NidhiKJha NidhiKJha deleted the fix-15104 branch November 10, 2022 03:45
@github-actions github-actions bot locked and limited conversation to collaborators Nov 10, 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: TagUrl
5 participants