[Cashtab] Move message signing and verification to new screen
AbandonedPublic
Actions

Authored by kieran709 on Nov 4 2022, 16:34.

Details

Reviewers

bytesofman
emack

Group Reviewers

Restricted Project

Summary

Related to T2734. Message signing and verification Collapses have been moved to their own page. A new component and route have been added, a new NavItem has been added to the hamburger menu, and snapshot testing has been implemented following the Send and SendToken template. This is the first time I have initialized a component with snapshots so I am expecting to make significant changes to SignVerigyMsg.test.js.

Test Plan

cd web/cashtab && npm start
expand the hamburger menu
navigate to Sign & Verify screen
ensure sign and verify UI and functionality is working

navigate to send screen
observe that the Sign and Verify Collapses are no longer present

npm test
observe that all tests pass

Diff Detail

Repository

rABC Bitcoin ABC

Branch

move-message-signing-verfication-to-new-screen

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 20807
Build 41273: Build Diff	cashtab-tests
Build 41272: arc lint + arc unit

Event Timeline

kieran709 created this revision.Nov 4 2022, 16:34

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 4 2022, 16:34

kieran709 requested review of this revision.Nov 4 2022, 16:34

Harbormaster completed remote builds in B20806: Diff 36132.Nov 4 2022, 16:37

Updated extension

Harbormaster completed remote builds in B20807: Diff 36133.Nov 4 2022, 16:48

Great diff!

See inline comment. I don't think App.js should be modified to pass these params to the new component, when other child components are not receiving these params from App.js
Need some styling work in the hamburger menu. The names should be centered in the left column. Sign and Verify length creates too much empty space to the left of other options.
This is a good working proof of concept diff. However, there is a lot going on here. This should be a stacked diff. Think about how you could divided this problem into smaller sub tasks and submit each one as part of a stack.

For example, something like

1 - add new icon
2 - remove from send.js
3 - add to new screen
4 - add snapshot tests

web/cashtab/src/components/SignVerifyMsg/SignVerifyMsg.js
75	It's probably worth studying what all should be pulled from context in App.js and then passed down to child components. Maybe getting settings in App.js and passing it down is best. However, none of the other screens do that right now. So, this one shouldn't either unless there is some reason unique to this new component where this is required.