Page MenuHomePhabricator

[Cashtab][Pt 1/4] Move message signing and verification to new screen - Create icon & NavItem
ClosedPublic

Authored by kieran709 on Nov 7 2022, 17:18.

Details

Summary

Related to T2734. Message signing and verification collapses should be on their own screen. Part one of this diff includes creating the custom icon component and adding the new option to the hamburger nav menu

Test Plan

cd web/cashtab && npm start
open the hamburger menu
observe that there is a new NavItem 'Sign & Verify Messages'

Per review feedback in D1241, styling has been fixed to center NavItem text.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
move-message-signing-verification-to-new-screen
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 20818
Build 41295: Build Diffcashtab-tests
Build 41294: arc lint + arc unit

Event Timeline

Updated styling and added the same changes to extension.

In general, if you have "and" in the title of your diff -- esp for a stacked diff --- it should be more than one diff

You actually have two changes here.

Since it's just styling and I get that it's a pretty big PITA to update a stacked diff with 4 parts so that part 1 is actually 2 parts, np for this time. But going forward, try to make every change as small as possible.

For example, this diff could be three diffs

  1. Adding the new custom icon (only change is to CustomIcons.js)
  2. Adding the custom icon to the menu for extension and normal + a stub page for sign + verify (just a template page)
  3. Fixing the formatting of the menu

It's almost impossible to make a diff that is "too small" as part of this kind of a stack. It simplifies the review process and also helps to organize your thinking about how the change is implemented.

This revision is now accepted and ready to land.Nov 8 2022, 11:34