fix: update css for modals #25961

sahar-fehri · 2024-07-19T07:19:24Z

Description

Few css updates on nft and token detection modal

Related issues

Fixes:

Manual testing steps

Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
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.

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.

github-actions · 2024-07-19T07:19:35Z

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.

codecov · 2024-07-22T14:15:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.66%. Comparing base (721a38b) to head (bd03784).
Report is 3 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25961      +/-   ##
===========================================
- Coverage    69.67%   69.66%   -0.01%     
===========================================
  Files         1402     1402              
  Lines        49652    49656       +4     
  Branches     13720    13719       -1     
===========================================
- Hits         34594    34591       -3     
- Misses       15058    15065       +7

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

metamaskbot · 2024-07-22T15:08:27Z

Builds ready [df37dea]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (145 ± 169 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	130	95	16	8
		domContentLoaded	9	59	27	14	7
		load	43	1674	145	351	169
		domInteractive	9	59	27	14	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: -18 Bytes (-0.00%)
common: 525 Bytes (0.01%)

…ops to change modal footer button styles and alignment

greptile-apps

PR Summary

CSS updates for NFT and token detection modals to enhance visual presentation and user experience.

ui/components/app/assets/auto-detect-nft/auto-detect-nft-modal.tsx: Removed textAlign from Text and Box components; added ButtonVariant to cancel button; updated ModalFooter containerProps to FlexDirection.ColumnReverse.
ui/components/app/assets/auto-detect-token/auto-detect-token-modal.tsx: Removed textAlign from Text and Box components; added style property to cancelButtonProps to remove border; added isConfirmButtonFirst to ModalFooter.

_{2 file(s) reviewed, 9 comment(s)}
_{Edit PR Review Bot Settings}

ui/components/app/assets/auto-detect-nft/auto-detect-nft-modal.tsx

ui/components/app/assets/auto-detect-token/auto-detect-token-modal.tsx

georgewrmarshall

LGTM! This is great use of design system components pushed a small update to prevent the need to update design system components. It also looks like a design pattern that we may want to promote so I'll create an issue to better document this in Figma and storybook. I really appreciate you reaching out and the before/after screenshots in the PR description. Thank you!

ui/components/app/assets/auto-detect-nft/auto-detect-nft-modal.tsx

greptile-apps

PR Summary

(updates since last review)

CSS updates for NFT and token detection modals to enhance visual presentation and user experience.

ui/components/app/assets/auto-detect-token/auto-detect-token-modal.tsx: Removed isConfirmButtonFirst property from ModalFooter component.
Ensure the removal of isConfirmButtonFirst does not affect button order or user experience in the modal.

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

georgewrmarshall

Re-reviewed, I missed the auto-detect-token modal on initital review. Left one suggestion

ui/components/app/assets/auto-detect-nft/auto-detect-nft-modal.tsx

ui/components/app/assets/auto-detect-token/auto-detect-token-modal.tsx

greptile-apps

PR Summary

(updates since last review)

CSS updates for NFT and token detection modals to enhance visual presentation and user experience.

ui/components/app/assets/auto-detect-nft/auto-detect-nft-modal.tsx: Removed padding from Text component and added variant to cancel button for consistent styling.
ui/components/app/assets/auto-detect-nft/auto-detect-nft-modal.tsx: Updated containerProps to use FlexDirection.ColumnReverse for improved layout.
ui/components/app/assets/auto-detect-token/auto-detect-token-modal.tsx: Removed textAlign from Text and Box components.
ui/components/app/assets/auto-detect-token/auto-detect-token-modal.tsx: Added ButtonVariant to cancelButtonProps to style the button as a link.

_{2 file(s) reviewed, 4 comment(s)}
_{Edit PR Review Bot Settings}

ui/components/app/assets/auto-detect-nft/auto-detect-nft-modal.tsx

ui/components/app/assets/auto-detect-token/auto-detect-token-modal.tsx

georgewrmarshall

Apologies again for further changes, but on third review, I noticed that we may want to duplicate the entire approach from the auto-detect-nft ModalFooter as it's likely intentional to have the same pattern on both modals.

greptile-apps

PR Summary

(updates since last review)

No major changes found since the last review.

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

sonarcloud · 2024-07-22T18:55:38Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
26.7% Duplication on New Code

See analysis details on SonarCloud

georgewrmarshall

LGTM!

checked AutoDetectNftModal button positioning in storybook ✅
checked AutoDetectTokenModal button positioning in storybook ✅

metamaskbot · 2024-07-22T19:23:22Z

Builds ready [bd03784]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (85 ± 14 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	193	111	29	14
		domContentLoaded	11	53	27	11	5
		load	46	170	85	30	14
		domInteractive	11	53	27	11	5

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 40 Bytes (0.00%)
common: 0 Bytes (0.00%)

fix: update css for modals

8251544

metamaskbot added the team-assets label Jul 19, 2024

sahar-fehri marked this pull request as ready for review July 19, 2024 15:59

sahar-fehri requested review from a team as code owners July 19, 2024 15:59

fix: fix lint

aba2429

sahar-fehri added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Jul 19, 2024

sahar-fehri added 2 commits July 19, 2024 18:36

fix: fix

6537ac0

Merge branch 'develop' into fix/token-nft-modal-css-update

df37dea

salimtb previously approved these changes Jul 22, 2024

View reviewed changes

chore: refactoring to revert modal-footer changes and use provided pr…

6ee2975

…ops to change modal footer button styles and alignment

georgewrmarshall dismissed salimtb’s stale review via 6ee2975 July 22, 2024 17:02

greptile-apps bot reviewed Jul 22, 2024

View reviewed changes

georgewrmarshall previously approved these changes Jul 22, 2024

View reviewed changes

ui/components/app/assets/auto-detect-nft/auto-detect-nft-modal.tsx Show resolved Hide resolved

fix: fix lint

1f9ce11

sahar-fehri dismissed georgewrmarshall’s stale review via 1f9ce11 July 22, 2024 17:22

greptile-apps bot reviewed Jul 22, 2024

View reviewed changes

georgewrmarshall reviewed Jul 22, 2024

View reviewed changes

fix: review

22a5066

greptile-apps bot reviewed Jul 22, 2024

View reviewed changes

georgewrmarshall reviewed Jul 22, 2024

View reviewed changes

fix: fix

bd03784

greptile-apps bot reviewed Jul 22, 2024

View reviewed changes

georgewrmarshall approved these changes Jul 22, 2024

View reviewed changes

bergeron approved these changes Jul 22, 2024

View reviewed changes

sahar-fehri merged commit 70955a5 into develop Jul 22, 2024
77 checks passed

sahar-fehri deleted the fix/token-nft-modal-css-update branch July 22, 2024 19:53

github-actions bot locked and limited conversation to collaborators Jul 22, 2024

metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update css for modals #25961

fix: update css for modals #25961

sahar-fehri commented Jul 19, 2024 •

edited

Loading

github-actions bot commented Jul 19, 2024

codecov bot commented Jul 22, 2024 •

edited

Loading

metamaskbot commented Jul 22, 2024

greptile-apps bot left a comment

georgewrmarshall left a comment

greptile-apps bot left a comment

georgewrmarshall left a comment

greptile-apps bot left a comment

georgewrmarshall left a comment

greptile-apps bot left a comment

sonarcloud bot commented Jul 22, 2024

georgewrmarshall left a comment

metamaskbot commented Jul 22, 2024

fix: update css for modals #25961

fix: update css for modals #25961

Conversation

sahar-fehri commented Jul 19, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jul 19, 2024

codecov bot commented Jul 22, 2024 • edited Loading

Codecov Report

metamaskbot commented Jul 22, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

georgewrmarshall left a comment

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

georgewrmarshall left a comment

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

georgewrmarshall left a comment

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

sonarcloud bot commented Jul 22, 2024

Quality Gate passed

georgewrmarshall left a comment

Choose a reason for hiding this comment

metamaskbot commented Jul 22, 2024

sahar-fehri commented Jul 19, 2024 •

edited

Loading

codecov bot commented Jul 22, 2024 •

edited

Loading