feat: display loader on re-designed confirmation page while blockaid validation is in progress #25477
Conversation
…tion is in progress
jpuri
added
confirmation-redesign
team-confirmations
|
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. |
Builds ready [f24c8be]
Page Load Metrics (51 ± 3 ms)
Bundle size diffs
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #25477 +/- ##
===========================================
+ Coverage 69.65% 69.66% +0.01%
===========================================
Files 1347 1348 +1
Lines 47803 47816 +13
Branches 13184 13188 +4
===========================================
+ Hits 33296 33309 +13
Misses 14507 14507 ☔ View full report in Codecov by Sentry. |
|
|
||
| const signatureSecurityAlertResponse = useSelector( | ||
| (state: SignatureSecurityAlertResponsesState) => | ||
| state.metamask.signatureSecurityAlertResponses?.[ |
There was a problem hiding this comment.
Since we also do this raw selector inside useBlockaidAlerts, is it worth a selector function for this?
There was a problem hiding this comment.
PR is updated to address this.
| if ( | ||
| signatureSecurityAlertResponse?.result_type === BlockaidResultType.Loading | ||
| ) { | ||
| return ( |
There was a problem hiding this comment.
Minor, but we could also early return null to avoid nesting the core logic of the component.
There was a problem hiding this comment.
PR is updated to address this.
| expect(container).toMatchSnapshot(); | ||
| }); | ||
|
|
||
| it('returns null if blockaid validaiton is not in progress', () => { |
There was a problem hiding this comment.
Minor, but would we benefit from a test to ensure it renders correctly with no Blockaid response at all?
There was a problem hiding this comment.
PR is updated to address this.
Builds ready [36e1a6c]
Page Load Metrics (52 ± 4 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| @@ -43,3 +48,23 @@ export const confirmSelector = (state: ConfirmMetamaskState) => state.confirm; | |||
|
|
|||
| export const currentConfirmationSelector = (state: ConfirmMetamaskState) => | |||
| state.confirm.currentConfirmation; | |||
|
|
|||
| export const currentSignatureRequestSecurityResponseSelector = ( | |||
There was a problem hiding this comment.
Doesn't need addressing now but we have lots of competing standards for selector names such as XSelector, getX, or selectX.
My understanding was the latter is best as it's the most explicit, makes auto-complete easy, and is recommended by Redux.
metamaskbot
added
the
release-12.1.0
Description
display loader on re-designed confirmation page while blockaid validation is in progress
Related issues
Fixes: #25323
Manual testing steps
Screenshots/Recordings
Pre-merge author checklist
Pre-merge reviewer checklist