feat: Support standalone confirmation for re-redesigned confirmations #13550

OGPoyraz · 2025-02-17T08:53:03Z

Description

This PR aims to implement support of standalone confirmation for redesigned confirmations.

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/4217

Manual testing steps

N/A

Screenshots/Recordings

Before

After

Pre-merge author checklist

I’ve followed MetaMask Contributor Docs and MetaMask Mobile 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.

OGPoyraz · 2025-02-17T10:33:33Z

Adding skip-sonar-cloud because throwing error is a known, expected and already created a task for upcoming work.

app/components/UI/Stake/routes/index.tsx

app/components/Views/confirmations/Confirm/ConfirmRoot.tsx

jpuri · 2025-02-17T11:11:20Z

app/components/Views/confirmations/components/Confirm/Info/StakingDeposit/Navbar.tsx

+      />
+    ),
+  };
+}


jpuri

I have some confusions in the PR, I added few questions.

jpuri · 2025-02-18T11:41:51Z

app/components/Views/confirmations/Confirm/Confirm.tsx

+        <ConfirmWrapped styles={styles} />
+      </View>
+    );
+  }


Should not StandaloneConfirmation be same as FlatConfirmation ?

No, because FlatConfirmation is a confirmation with fullscreen that's all.

One example of FlatConfirmation will be SendFlow, where it will have it's own Stack.Navigator and Stack.Screens.

In summary;

ModalConfirmations => for using Signatures, all other Transaction types etc
FlatConfirmation => A confirmation where it needs to be a full flow, has it's own navigator inside
StandaloneConfirmation => Where we can put a confirmation anywhere in the app (In any Stack)

I think these covering all the use cases we have it in mobile today

But I think we need only 2 types of styling, which essentially is point of classification here - modal and full screen.

amitabh94 · 2025-02-18T16:29:58Z

Why are we not running e2e tests on this ? We should run the staking e2e tests atleast so as to avoid any regressions.

I see from staking point of view you are just adding a route but would feel more comfortable approving this if we can run the tests.

OGPoyraz · 2025-02-18T16:54:41Z

@amitabh94 if you are asking the label no smoke e2e - we could run (I believe this should be default, not tied to any label as in extension), but as we all know this has no affect in the current flows it felt a bit unnecessary for now. (considering we've been told several times that running e2e tests are spending lot of credits on Bitrise) Once we toggled on / start using new flow - as expected there will be no PR with that label. But since this is asked I will do that now.

Update: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f9e83941-fe03-430a-a9a6-4f328860da44?tab=workflows
It works properly for staking as expected.

If you are asking of why redesigned staking deposit confirmation not e2e tested, this has some reasons.

Not sure if you can see the board but this is already in the upcoming tasks to handle e2e (https://github.com/orgs/MetaMask/projects/78?pane=issue&itemId=95989587)
We are trying to keep iterations with small PRs so easier to review
Staking deposit is the first re-designed transaction confirmation so first PRs in this effort is settling lot of infrastructure required by our team

github-actions · 2025-02-18T16:56:03Z

Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 0768be2
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/30ab5a89-9d49-462d-b64d-3019f895a8c8

Note

This comment will auto-update when build completes
You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions · 2025-02-18T16:58:25Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: a00e484
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f9e83941-fe03-430a-a9a6-4f328860da44

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions · 2025-02-19T11:49:34Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 51084ca
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/72fd88dc-c4f9-4ec5-b81a-55f42a8a68ba

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

sonarqubecloud · 2025-02-19T12:00:25Z

Quality Gate failed

Failed conditions
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

jpuri · 2025-02-20T04:12:44Z

app/components/Views/confirmations/components/Confirm/Title/Title.tsx

@@ -60,9 +61,14 @@ const Title = () => {
  const { approvalRequest } = useApprovalRequest();
  const signatureRequest = useSignatureRequest();
  const { styles } = useStyles(styleSheet, {});
+  const { isStandaloneConfirmation } = useStandaloneConfirmation();


We should be able to use useFlatConfirmation here.

jpuri · 2025-02-20T04:38:56Z

app/components/Views/confirmations/Confirm/ConfirmRoot.test.tsx

-    expect(mockNavigate).toHaveBeenCalledTimes(1);
-    expect(mockNavigate).toHaveBeenCalledWith(Routes.CONFIRM_FLAT_PAGE);
-  });
+  // TODO: Add unit test for once we have any existing flat confirmation


Let's not remove the test case till code is there.

jpuri · 2025-02-20T04:39:52Z

Hey @OGPoyraz : let's try to enable sonar again, the issue might already be fixed.

OGPoyraz requested review from a team as code owners February 17, 2025 08:53

metamaskbot added the team-confirmations Push issues to confirmations team label Feb 17, 2025

pedronfigueiredo previously approved these changes Feb 17, 2025

View reviewed changes

OGPoyraz dismissed pedronfigueiredo’s stale review via 1f5f2dc February 17, 2025 10:00

OGPoyraz added the skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. label Feb 17, 2025

OGPoyraz added the No E2E Smoke Needed If the PR does not need E2E smoke test run label Feb 17, 2025

pedronfigueiredo previously approved these changes Feb 17, 2025

View reviewed changes

OGPoyraz enabled auto-merge February 17, 2025 10:48

jpuri reviewed Feb 17, 2025

View reviewed changes

app/components/UI/Stake/routes/index.tsx Show resolved Hide resolved

jpuri reviewed Feb 17, 2025

View reviewed changes

app/components/Views/confirmations/Confirm/ConfirmRoot.tsx Show resolved Hide resolved

jpuri reviewed Feb 17, 2025

View reviewed changes

jpuri requested changes Feb 17, 2025

View reviewed changes

OGPoyraz dismissed pedronfigueiredo’s stale review via 32dcc95 February 17, 2025 11:33

OGPoyraz disabled auto-merge February 17, 2025 11:33

OGPoyraz requested a review from jpuri February 17, 2025 11:48

jpuri reviewed Feb 18, 2025

View reviewed changes

OGPoyraz added 11 commits February 18, 2025 13:02

Implement standalone confirmation

11fe037

Update

64a59a6

Update

8902416

Update

9fb7532

Fix lint

b58b19f

Add todo

72c80f0

fix todo

3f7c96f

Update

3c27fe7

Update

0106769

Fix lint

f6eba09

Rephrase test

fbf6468

OGPoyraz added 6 commits February 18, 2025 13:03

Fix background color

70151a2

Fix unit test

c5ba6d5

Add staking deposit tests

9ea208f

Fix test

d8bcaf9

Update standalone component hierarchy

514c523

Fix test

65f59d2

OGPoyraz force-pushed the ogp/standalone-poc branch from 76e0f91 to 65f59d2 Compare February 18, 2025 12:05

OGPoyraz added 3 commits February 18, 2025 13:06

Fix lint

ca93177

Fix lint

307ab9b

Merge branch 'main' into ogp/standalone-poc

d42e716

OGPoyraz added 2 commits February 18, 2025 17:49

Update

ceaf463

Fix tests

0768be2

OGPoyraz added Run Smoke E2E Triggers smoke e2e on Bitrise and removed No E2E Smoke Needed If the PR does not need E2E smoke test run labels Feb 18, 2025

OGPoyraz added 2 commits February 18, 2025 17:57

Fix lint

916f452

Merge branch 'main' into ogp/standalone-poc

a00e484

OGPoyraz added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Feb 18, 2025

Merge branch 'main' into ogp/standalone-poc

51084ca

OGPoyraz added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Feb 19, 2025

jpuri reviewed Feb 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support standalone confirmation for re-redesigned confirmations #13550

feat: Support standalone confirmation for re-redesigned confirmations #13550

OGPoyraz commented Feb 17, 2025

OGPoyraz commented Feb 17, 2025 •

edited

Loading

jpuri Feb 17, 2025

jpuri left a comment

jpuri Feb 18, 2025

OGPoyraz Feb 18, 2025

OGPoyraz Feb 18, 2025 •

edited

Loading

jpuri Feb 18, 2025

amitabh94 commented Feb 18, 2025

OGPoyraz commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025

github-actions bot commented Feb 18, 2025 •

edited by metamaskbot

Loading

github-actions bot commented Feb 19, 2025 •

edited by metamaskbot

Loading

sonarqubecloud bot commented Feb 19, 2025

jpuri Feb 20, 2025

jpuri Feb 20, 2025

jpuri commented Feb 20, 2025

feat: Support standalone confirmation for re-redesigned confirmations #13550

Are you sure you want to change the base?

feat: Support standalone confirmation for re-redesigned confirmations #13550

Conversation

OGPoyraz commented Feb 17, 2025

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

OGPoyraz commented Feb 17, 2025 • edited Loading

jpuri Feb 17, 2025

Choose a reason for hiding this comment

jpuri left a comment

Choose a reason for hiding this comment

jpuri Feb 18, 2025

Choose a reason for hiding this comment

OGPoyraz Feb 18, 2025

Choose a reason for hiding this comment

OGPoyraz Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

jpuri Feb 18, 2025

Choose a reason for hiding this comment

amitabh94 commented Feb 18, 2025

OGPoyraz commented Feb 18, 2025 • edited Loading

github-actions bot commented Feb 18, 2025

Bitrise

github-actions bot commented Feb 18, 2025 • edited by metamaskbot Loading

Bitrise

github-actions bot commented Feb 19, 2025 • edited by metamaskbot Loading

Bitrise

sonarqubecloud bot commented Feb 19, 2025

Quality Gate failed

jpuri Feb 20, 2025

Choose a reason for hiding this comment

jpuri Feb 20, 2025

Choose a reason for hiding this comment

jpuri commented Feb 20, 2025

OGPoyraz commented Feb 17, 2025 •

edited

Loading

OGPoyraz Feb 18, 2025 •

edited

Loading

OGPoyraz commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025 •

edited by metamaskbot

Loading

github-actions bot commented Feb 19, 2025 •

edited by metamaskbot

Loading