fix: Hide MMI Account Mistmatch BannerAlert from Sign-in with Ethereum (SIWE) Redesign Page #25662

digiwand · 2024-07-03T16:38:10Z

Description

Related issues

Fixes: #25661

Manual testing steps

Go to test-dapp
Connect with an account
Open MM and change the account
Test SIWE and observe no MMI account mismatch banner. Bad Account should still display account mismatch row alerts

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-03T16:38:21Z

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-03T21:57:12Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.80%. Comparing base (5ee57a6) to head (e81f316).
Report is 8 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25662      +/-   ##
===========================================
+ Coverage    69.77%   69.80%   +0.02%     
===========================================
  Files         1376     1377       +1     
  Lines        48403    48437      +34     
  Branches     13348    13355       +7     
===========================================
+ Hits         33773    33807      +34     
  Misses       14630    14630

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

metamaskbot · 2024-07-03T22:09:18Z

Builds ready [3689366]

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, 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 (257 ± 235 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	386	129	88	42
		domContentLoaded	10	96	35	26	12
		load	45	1771	257	490	235
		domInteractive	10	96	35	26	12

Bundle size diffs

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

matthewwalsh0 · 2024-07-05T20:59:07Z

ui/components/institutional/signature-mismatch-banner/signature-mismatch-banner.test.tsx

+ },
+ });
+
+ expect(container.querySelector('.mm-banner-alert')).not.toBeInTheDocument();


Minor, but for consistency should we use the same selector as above in the positive test? Or change both to classes?

hi @matthewwalsh0! Thanks for the suggestion

using getByText and not retrieving a result would result in an error

TestingLibraryElementError: Unable to find an element with the text:

I do like the test above since it checks the content. I added the class check to the test above to display the inverse checks more explicitly

sonarcloud · 2024-07-09T12:00:30Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

metamaskbot · 2024-07-09T12:45:23Z

Builds ready [e81f316]

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, 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 (257 ± 258 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	71	337	120	54	26
		domContentLoaded	10	111	33	22	10
		load	44	2090	257	537	258
		domInteractive	10	111	33	22	10

Bundle size diffs

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

digiwand requested a review from a team as a code owner July 3, 2024 16:38

metamaskbot added the team-confirmations Push issues to confirmations team label Jul 3, 2024

digiwand linked an issue Jul 3, 2024 that may be closed by this pull request

[Signature Redesign]: Disable MMI account mismatch for SIWE pages #25661

Closed

9 tasks

Base automatically changed from MMI-5202-account-mismatch-banner to develop July 4, 2024 09:28

albertolive requested review from a team as code owners July 4, 2024 09:28

feat: hide MMI account mistmatch alert for SIWE redesign

8fa1cef

digiwand force-pushed the fix-hide-siwe-MMI-account-banner branch from 3689366 to 8fa1cef Compare July 5, 2024 15:28

refactor: use isSIWESignatureRequest

23773bb

matthewwalsh0 previously approved these changes Jul 5, 2024

View reviewed changes

test: MMISignatureMismatchBanner test class exists

e81f316

digiwand dismissed matthewwalsh0’s stale review via e81f316 July 9, 2024 11:58

digiwand changed the title ~~Hide MMI Account Mistmatch BannerAlert from Sign-in with Ethereum (SIWE) Redesign Page~~ fix: Hide MMI Account Mistmatch BannerAlert from Sign-in with Ethereum (SIWE) Redesign Page Jul 9, 2024

jpuri approved these changes Jul 9, 2024

View reviewed changes

pedronfigueiredo approved these changes Jul 9, 2024

View reviewed changes

zone-live approved these changes Jul 9, 2024

View reviewed changes

digiwand merged commit 19b0e57 into develop Jul 9, 2024
74 of 75 checks passed

digiwand deleted the fix-hide-siwe-MMI-account-banner branch July 9, 2024 13:29

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Hide MMI Account Mistmatch BannerAlert from Sign-in with Ethereum (SIWE) Redesign Page #25662

fix: Hide MMI Account Mistmatch BannerAlert from Sign-in with Ethereum (SIWE) Redesign Page #25662

digiwand commented Jul 3, 2024 •

edited

Loading

github-actions bot commented Jul 3, 2024

codecov bot commented Jul 3, 2024 •

edited

Loading

metamaskbot commented Jul 3, 2024

matthewwalsh0 Jul 5, 2024

digiwand Jul 9, 2024

sonarcloud bot commented Jul 9, 2024

metamaskbot commented Jul 9, 2024

fix: Hide MMI Account Mistmatch BannerAlert from Sign-in with Ethereum (SIWE) Redesign Page #25662

fix: Hide MMI Account Mistmatch BannerAlert from Sign-in with Ethereum (SIWE) Redesign Page #25662

Conversation

digiwand commented Jul 3, 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 3, 2024

codecov bot commented Jul 3, 2024 • edited Loading

Codecov Report

metamaskbot commented Jul 3, 2024

matthewwalsh0 Jul 5, 2024

Choose a reason for hiding this comment

digiwand Jul 9, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Jul 9, 2024

Quality Gate passed

metamaskbot commented Jul 9, 2024

digiwand commented Jul 3, 2024 •

edited

Loading

codecov bot commented Jul 3, 2024 •

edited

Loading