Commit a9c779f
authored
fix: qr scanner appearing before the confirmations screen cp-7.77.0 (#30088)
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until this PR meets the canonical
Definition of Ready For Review in `docs/readme/ready-for-review.md`.
In short: the template must be materially complete (not just section
titles
present), all status checks must be currently passing, and the only
expected
follow-up commits must be reviewer-driven.
-->
## **Description**
This PR fixes the issue where the qr scanning appears before the
confirmation screen.
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
## **Changelog**
<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`
If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`
(This helps the Release Engineer do their job more quickly and
accurately)
-->
CHANGELOG entry: fixes the the qr scanner appearing before confirmations
page
## **Related issues**
Fixes: #29949
## **Manual testing steps**
```gherkin
Scenario: QR hardware wallet transaction shows confirmation screen first
Given the user has a QR hardware wallet account (e.g. Keystone) set up
And the user is on that QR account in MetaMask
When the user initiates a send transaction to an address
Then the confirmation screen is displayed (NOT the QR scanner)
And the user sees transaction details (amount, recipient, gas)
Scenario: QR hardware wallet user confirms a transaction
Given the user is on the confirmation screen for a QR account transaction
When the user taps "Confirm"
Then the QR scanner screen appears
And the app waits for the QR code scan to complete the transaction
Scenario: QR hardware wallet user rejects a transaction
Given the user is on the confirmation screen for a QR account transaction
When the user taps "Reject"
Then the transaction is cancelled
And the QR scanner does NOT appear
Scenario: QR hardware wallet signature request shows confirmation first
Given the user is on a QR hardware wallet account
When a dApp requests a signature (personal_sign or eth_signTypedData)
Then the confirmation screen is displayed
And the QR scanner does NOT appear until the user taps "Confirm"
Scenario: QR hardware wallet signature request shows QRInfo after confirm
Given the user is on the confirmation screen for a QR account signature request
When the user taps "Confirm"
Then the QR scanner / QRInfo view is shown
And the user can scan the QR code to complete signing
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
### **After**
https://github.com/user-attachments/assets/ee6004b2-2f63-47b2-b007-d623b7ed990e
https://github.com/user-attachments/assets/9798e0f2-6756-4e4b-87fd-ee2cc1ebf98b
<!-- [screenshots/recordings] -->
## **Pre-merge author checklist**
<!--
Every checklist item must be consciously assessed before marking this PR
as
"Ready for review". A checked box means you deliberately considered that
responsibility, not that you literally performed every action listed.
Unchecked boxes are ambiguous: they are not an implicit "N/A" and they
are not
a silent "skip". See `docs/readme/ready-for-review.md` for the full
checklist
semantics.
-->
- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
#### Performance checks (if applicable)
- [ ] I've tested on Android
- Ideally on a mid-range device; emulator is acceptable
- [ ] I've tested with a power user scenario
- Use these [power-user
SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93)
to import wallets with many accounts and tokens
- [ ] I've instrumented key operations with Sentry traces for production
performance metrics
- See [`trace()`](/app/util/trace.ts) for usage and
[`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274)
for an example
For performance guidelines and tooling, see the [Performance
Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers).
## **Pre-merge reviewer checklist**
<!--
Reviewer checklist items follow the same semantics as the author
checklist: an
unchecked box is ambiguous, a checked box means the reviewer consciously
assessed that responsibility. See `docs/readme/ready-for-review.md`.
-->
- [ ] 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.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes confirmation submit flow for QR hardware wallets by adding new
state and altering when scanning UI is shown; mistakes could block or
prematurely trigger signing for QR accounts. Scope is contained to
confirmations/QR hardware paths with updated tests.
>
> **Overview**
> Prevents the QR scanner/`QRInfo` view from appearing before the
confirmations screen by introducing `signingConfirmed` in
`QRHardwareContext` and only rendering `QRInfo` when QR signing is
active *and* the user has confirmed.
>
> Updates `useConfirmActions` so tapping Confirm during an in-progress
QR signing session sets `signingConfirmed` and explicitly shows the
scanner, while still delegating to `useQrConfirm` for QR-hardware
accounts; also ensures `signingConfirmed` is set before executing the
default approval/transaction paths.
>
> Adjusts QR confirm UX so `useQrConfirm` re-shows the
awaiting-confirmation bottom sheet when QR signing is already in
progress (instead of opening the `QRInfo` scanner), and updates footer
behavior/tests so the button label remains `Confirm` during QR signing.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
864f724. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->1 parent e3a3c5f commit a9c779f
11 files changed
Lines changed: 138 additions & 47 deletions
File tree
- app
- components/Views/confirmations
- components
- info-root
- context/qr-hardware-context
- hooks
- send
- core/HardwareWallet/hooks
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
160 | 160 | | |
161 | 161 | | |
162 | 162 | | |
163 | | - | |
| 163 | + | |
164 | 164 | | |
165 | 165 | | |
166 | 166 | | |
167 | 167 | | |
168 | 168 | | |
169 | 169 | | |
170 | | - | |
| 170 | + | |
171 | 171 | | |
172 | 172 | | |
173 | 173 | | |
| |||
Lines changed: 2 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
| 42 | + | |
42 | 43 | | |
43 | 44 | | |
44 | 45 | | |
| |||
61 | 62 | | |
62 | 63 | | |
63 | 64 | | |
64 | | - | |
| 65 | + | |
65 | 66 | | |
66 | 67 | | |
67 | 68 | | |
| |||
121 | 122 | | |
122 | 123 | | |
123 | 124 | | |
124 | | - | |
125 | | - | |
126 | | - | |
127 | | - | |
128 | 125 | | |
129 | 126 | | |
130 | 127 | | |
| |||
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
174 | 174 | | |
175 | 175 | | |
176 | 176 | | |
177 | | - | |
| 177 | + | |
178 | 178 | | |
179 | 179 | | |
| 180 | + | |
180 | 181 | | |
181 | 182 | | |
182 | 183 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
91 | | - | |
| 91 | + | |
92 | 92 | | |
93 | 93 | | |
94 | 94 | | |
| |||
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
105 | | - | |
| 105 | + | |
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
| |||
Lines changed: 3 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
105 | 105 | | |
106 | 106 | | |
107 | 107 | | |
108 | | - | |
| 108 | + | |
109 | 109 | | |
110 | 110 | | |
111 | 111 | | |
| |||
229 | 229 | | |
230 | 230 | | |
231 | 231 | | |
232 | | - | |
| 232 | + | |
233 | 233 | | |
234 | 234 | | |
235 | 235 | | |
| |||
246 | 246 | | |
247 | 247 | | |
248 | 248 | | |
249 | | - | |
250 | | - | |
251 | | - | |
252 | | - | |
| 249 | + | |
253 | 250 | | |
254 | 251 | | |
255 | 252 | | |
| |||
Lines changed: 7 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
| 26 | + | |
25 | 27 | | |
26 | 28 | | |
27 | 29 | | |
| |||
33 | 35 | | |
34 | 36 | | |
35 | 37 | | |
| 38 | + | |
| 39 | + | |
36 | 40 | | |
37 | 41 | | |
38 | 42 | | |
| |||
43 | 47 | | |
44 | 48 | | |
45 | 49 | | |
| 50 | + | |
46 | 51 | | |
47 | 52 | | |
48 | 53 | | |
| |||
87 | 92 | | |
88 | 93 | | |
89 | 94 | | |
| 95 | + | |
| 96 | + | |
90 | 97 | | |
91 | 98 | | |
92 | 99 | | |
| |||
Lines changed: 4 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | | - | |
| 80 | + | |
81 | 81 | | |
82 | 82 | | |
83 | 83 | | |
84 | 84 | | |
85 | 85 | | |
86 | | - | |
| 86 | + | |
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
93 | | - | |
| 93 | + | |
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
114 | | - | |
| 114 | + | |
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
| |||
Lines changed: 76 additions & 15 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
39 | 43 | | |
40 | 44 | | |
41 | 45 | | |
| |||
91 | 95 | | |
92 | 96 | | |
93 | 97 | | |
94 | | - | |
| 98 | + | |
95 | 99 | | |
96 | 100 | | |
97 | 101 | | |
98 | 102 | | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | | - | |
103 | | - | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
104 | 110 | | |
105 | 111 | | |
106 | 112 | | |
107 | | - | |
108 | | - | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
109 | 117 | | |
110 | 118 | | |
111 | 119 | | |
112 | 120 | | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
113 | 126 | | |
114 | | - | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
115 | 137 | | |
116 | 138 | | |
117 | | - | |
118 | | - | |
119 | | - | |
120 | | - | |
121 | | - | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
122 | 142 | | |
123 | 143 | | |
124 | 144 | | |
| |||
129 | 149 | | |
130 | 150 | | |
131 | 151 | | |
132 | | - | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
133 | 194 | | |
134 | 195 | | |
135 | 196 | | |
| |||
Lines changed: 21 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
| |||
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
26 | | - | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
27 | 33 | | |
28 | 34 | | |
29 | 35 | | |
| |||
85 | 91 | | |
86 | 92 | | |
87 | 93 | | |
88 | | - | |
| 94 | + | |
89 | 95 | | |
90 | 96 | | |
91 | 97 | | |
| |||
95 | 101 | | |
96 | 102 | | |
97 | 103 | | |
98 | | - | |
99 | | - | |
100 | | - | |
101 | | - | |
| 104 | + | |
| 105 | + | |
102 | 106 | | |
103 | 107 | | |
104 | 108 | | |
| |||
111 | 115 | | |
112 | 116 | | |
113 | 117 | | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
114 | 124 | | |
| 125 | + | |
115 | 126 | | |
116 | 127 | | |
117 | 128 | | |
118 | 129 | | |
| 130 | + | |
119 | 131 | | |
120 | 132 | | |
121 | 133 | | |
122 | 134 | | |
| 135 | + | |
123 | 136 | | |
| 137 | + | |
| 138 | + | |
124 | 139 | | |
125 | 140 | | |
126 | 141 | | |
| |||
0 commit comments