Skip to content

feat: create ConfirmTitle component#22270

Merged
segun merged 24 commits intodevelopfrom
feat-confirm-title-with-description-2
Jan 30, 2024
Merged

feat: create ConfirmTitle component#22270
segun merged 24 commits intodevelopfrom
feat-confirm-title-with-description-2

Conversation

@digiwand
Copy link
Copy Markdown
Contributor

@digiwand digiwand commented Dec 13, 2023 •
edited
Loading

Description

New component to display the confirmation title and subtitle

Notes:

  • storybook page created
  • I put the custom hooks directly in the confirm/title directory because it is not being used outside of this directory

Related issues

Fixes: #21282
Blocked By: #21970

Manual testing steps

Screenshots/Recordings

see blue box in below image:
confirmtitle

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@digiwand digiwand requested a review from a team as a code owner December 13, 2023 16:30
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 13, 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.

@digiwand digiwand mentioned this pull request Dec 13, 2023
Closed
13 tasks
@digiwand digiwand requested a review from jpuri December 14, 2023 16:15
jpuri
jpuri previously approved these changes Dec 15, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

👍

digiwand
digiwand commented Dec 18, 2023
View reviewed changes
@digiwand digiwand requested a review from jpuri December 19, 2023 15:22
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Dec 19, 2023

Builds ready [d37e799]
Page Load Metrics (1231 ± 113 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint942591586330
domContentLoaded10175636933
load85016861231236113
domInteractive10175636933
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 180 Bytes (0.00%)

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 19, 2023 •
edited
Loading

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (cd38cdb) 68.08% compared to head (a688af3) 68.10%.

❗ Current head a688af3 differs from pull request most recent head 03a4f09. Consider uploading reports for the commit 03a4f09 to get more accurate results

Files Patch % Lines
ui/components/app/confirm/title/title.tsx 90.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #22270      +/-   ##
===========================================
+ Coverage    68.08%   68.10%   +0.01%     
===========================================
  Files         1091     1092       +1     
  Lines        42923    42927       +4     
  Branches     11415    11413       -2     
===========================================
+ Hits         29223    29232       +9     
+ Misses       13700    13695       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jpuri
jpuri previously approved these changes Dec 19, 2023
View reviewed changes
georgewrmarshall
georgewrmarshall reviewed Jan 10, 2024
View reviewed changes
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 18, 2024

Builds ready [8f0d4f9]
Page Load Metrics (1255 ± 96 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint922761435929
domContentLoaded10198506431
load8471571125520096
domInteractive10198506431
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 179 Bytes (0.00%)

segun
segun suggested changes Jan 19, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@segun segun left a comment

Choose a reason for hiding this comment

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

Would be nice to add storybook as well.

@digiwand
Copy link
Copy Markdown
Contributor Author

digiwand commented Jan 20, 2024

hi @segun, thanks for the review!

I was holding off on creating storybook pages for this reason:

no storybook page created for this. We can display this within the confirm page storybook page(s) when we create them. see: https://storybook.js.org/docs/writing-stories/build-pages-with-storybook#args-composition-for-presentational-screens

however, I've added it now upon request since it could be useful. It's a simple storybook page for now, so it doesn't have controls yet we don't support other variants at this time

updated 5983f88

@digiwand digiwand requested a review from segun January 20, 2024 02:19
@georgewrmarshall
Copy link
Copy Markdown
Contributor

georgewrmarshall commented Jan 20, 2024

hi @segun, thanks for the review!

I was holding off on creating storybook pages for this reason:

no storybook page created for this. We can display this within the confirm page storybook page(s) when we create them. see: https://storybook.js.org/docs/writing-stories/build-pages-with-storybook#args-composition-for-presentational-screens

however, I've added it now upon request since it could be useful. It's a simple storybook page for now, so it doesn't have controls yet we don't support other variants at this time

updated 5983f88

I also think a storybook file no matter how simple the component is can be helpful :)

@digiwand
Copy link
Copy Markdown
Contributor Author

digiwand commented Jan 22, 2024

gotchya, thanks for the direction on storybook pages here @segun & @georgewrmarshall!

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 22, 2024

Builds ready [a7b7d2b]
Page Load Metrics (1237 ± 149 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint813351477235
domContentLoaded8234517234
load75819601237310149
domInteractive8234517234
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 179 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 25, 2024

Builds ready [4033482]
Page Load Metrics (809 ± 34 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint88148114168
domContentLoaded105217105
load73310318097134
domInteractive105217105
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 179 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 29, 2024

Builds ready [a688af3]
Page Load Metrics (733 ± 10 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8411810173
domContentLoaded9211631
load6827667332210
domInteractive9211631
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 179 Bytes (0.00%)

@digiwand
Copy link
Copy Markdown
Contributor Author

digiwand commented Jan 30, 2024

Screenshot 2024-01-29 at 4 41 00 PM

hi @segun,
not seeing a way to dismiss or mark the change request as complete. may I get a rereview please?

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 30, 2024

Builds ready [03a4f09]
Page Load Metrics (779 ± 15 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint93137111147
domContentLoaded9291552
load7288377793215
domInteractive9291552
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 179 Bytes (0.00%)

@segun segun merged commit ea23260 into develop Jan 30, 2024
@segun segun deleted the feat-confirm-title-with-description-2 branch January 30, 2024 20:09
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2024
@metamaskbot metamaskbot added the release-11.11.0 Issue or pull request that will be included in release 11.11.0 label Jan 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jpuri jpuri jpuri approved these changes

@georgewrmarshall georgewrmarshall georgewrmarshall approved these changes

@blackdevelopa blackdevelopa Awaiting requested review from blackdevelopa

+1 more reviewer

@segun segun segun approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

confirmation-redesign release-11.11.0 Issue or pull request that will be included in release 11.11.0 team-confirmations-secure-ux-PR PRs from the confirmations team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Create a component to show title and description of confirmation pages

6 participants

@digiwand @metamaskbot @georgewrmarshall @segun @jpuri @blackdevelopa