Address book send plus contact list #6914

danjm · 2019-07-25T14:58:42Z

Resolves #6365
Resolve #6585
Closes #6702, #6681

This PR includes all the work done #6681 and #6702. These PRs included the new address book send flow and much of the work for the contact list settings feature.

In addition to that work, this PR adds a number of changes and improvements have been made to the address book settings work, including:

correct header/sub-header names on all screens in address book settings, such that viewing profiles with and without nicknames will show appropriate headers
adds the "My Accounts" screen
changes the add button to the floating circular button
updates styles to match https://www.figma.com/proto/giaffk6qfl9eMMjoPFjJkGnt/Address-book?node-id=0%3A1&scaling=min-zoom, (the most significant change in that regard is to use the same contact list as used in the send screen)
adds ENS resolution (same as on the send screen) to the settings address book

Demo video here: https://streamable.com/lcdfd

@whymarrh @jennypollack @cjeria @bdresser

metamaskbot · 2019-07-25T15:22:57Z

Builds ready [fbb2fa9]: chrome, firefox, edge, opera

whymarrh

A few changes. I'll take another look once a few of them have been implemented.

ui/app/components/ui/dialog/index.js

whymarrh · 2019-07-25T16:07:42Z

ui/app/pages/send/send-content/add-recipient/add-recipient.component.js

+
+    if (query) {
+      if (!this.contactFuse) {
+        this.contactFuse = new Fuse(contacts, {


This might be better off attached to the class itself as a property so that we don't need to manage the state ourselves—as a property it would be created once when the class is constructed.

whymarrh · 2019-07-25T16:14:50Z

ui/app/pages/send/to-autocomplete/to-autocomplete.js

-      style: { color: '#dedede' },
-      onClick: () => this.handleInputEvent(),
-    }),
+    // !to && h(`i.fa.fa-caret-down.fa-lg.send-v2__to-autocomplete__down-caret`, {


Nit: we can remove these lines

whymarrh · 2019-07-25T16:18:39Z

ui/app/pages/settings/contact-list-tab/view-contact/view-contact.component.js

+import Identicon from '../../../../components/ui/identicon'
+import { CONTACT_LIST_ROUTE, CONTACT_EDIT_ROUTE } from '../../../../helpers/constants/routes'
+import Button from '../../../../components/ui/button/button.component'
+const copyToClipboard = require('copy-to-clipboard')


Nit: import copyToClipboard from 'copy-to-clipboard' maybe

whymarrh · 2019-07-25T16:20:16Z

ui/app/store/actions.js

+  }
+}
+
+// Calls the addressBookController to remove an existing address.


Nit: Can we use JSDoc here?

whymarrh · 2019-07-25T16:22:28Z

ui/app/selectors/selectors.js

+function getAddressBookEntryName (state, address) {
+  const entry = getAddressBookEntry(state, address) || state.metamask.identities[address]
+  const name = entry && entry.name !== '' ? entry.name : addressSlicer(address)
+  return name


Nit: inline this return statement

whymarrh · 2019-07-25T16:26:48Z

ui/app/selectors/selectors.js

@@ -203,7 +208,26 @@ function conversionRateSelector (state) {
 }

 function getAddressBook (state) {
-  return state.metamask.addressBook
+  const network = state.metamask.network
+  const addressBookEntries = Object.entries(state.metamask.addressBook)


This can probably be Object.values and then filtered

whymarrh · 2019-07-25T16:27:17Z

ui/app/pages/settings/settings.container.js

+    backRoute = SETTINGS_ROUTE
+  }
+
+  const address = pathname.slice(-42)


Where does 42 come from?

We could instead treat pathname here as a URL component and split it on / and pick out the constituent parts which might be less fragile?

whymarrh · 2019-07-25T16:33:40Z

ui/app/pages/send/send-content/add-recipient/ens-input.component.js

+const copyToClipboard = require('copy-to-clipboard/index')
+const ENS = require('ethjs-ens')
+const networkMap = require('ethjs-ens/lib/network-map.json')
+const log = require('loglevel')


Can we translate all of these to ESM imports?

whymarrh · 2019-07-25T16:56:36Z

Also we'll need to rebase this on the latest develop

danjm · 2019-07-26T00:14:54Z

@whymarrh Apart from the one comment I directly replied to, all of your comments are addressed in 305e96a1d

I also addressed all the other issues we found earlier while QAing in the following commits:
6653e60f4 Fix send screen contact searching after network switching
07ba4c2e3 Fix display of all recents after clicking 'Load More' in contact list
a53563491 Ensure add-contact component properly updates after QR code detection
d7e599ced Use keyCode to detect enter key in AddToAddressBookModal
1b709c0de Default alias state prop in AddToAddressBookModal to empty string
e81e902d4 Make the displayed and copied address in view-contact.component the checksummed address
4e122c3a5 Show resolved ens icon and address if exists (settings: add-contact.component)
3ccc5e8de Resolve send screen search for ensAddress to matching address book entry if it exists
d7c444f9b Switching networks when an ens address is shown on send form removes the ens address.

metamaskbot · 2019-07-26T02:38:45Z

Builds ready [1fa7533]: chrome, firefox, edge, opera

danjm · 2019-07-26T02:47:43Z

Rebased onto develop

metamaskbot · 2019-07-26T03:52:53Z

Builds ready [a8439a8]: chrome, firefox, edge, opera

metamaskbot · 2019-07-26T13:20:39Z

Builds ready [1b6430d]: chrome, firefox, edge, opera

cjeria · 2019-07-30T04:36:22Z

Here's my design feedback for the address book.

ENS functionality

ENS error message copy

Can be made clearer by explicitly saying "ENS name not found on the current network. Try switching to Main Ethereum Network".

ENS address not displaying (bug)

Not sure what's going on but I have an ENS name that I've registered and set a resolver address (my full name) and the result is "0x..0x". As you can see it throw a check indicating that it recognizes this ENS name as an address, but it doesn't display the actual address under the name as I'd expect it to. Let me know if you need any more info to help debug this.

Contact list

Cancelling out of Edit Mode

When editing a contact there is a "save" and “cancel” button. The cancel button should take you out of “edit” mode, NOT take you back to the full contact list.

Memo

The memo is missing - I personally think it’s a nice feature to help add context to accounts, I’d push to add this in.

Current implementation

Designs (see specs in Figma file)

Fullscreen mode

I'd suggest we do a two-column layout as was done for networks in settings. I quickly hacked this screen together to get the idea across. let me know if you want me to provide more direction here.

I realize "My wallet accounts" is 3 levels deep, so I'd like to suggest we introduce breadcrumbs when we have a nav >2 levels deep in settings. Again, I just hacked these screens together, so let me know if you have any questions.

Designs are in here)

danjm · 2019-07-30T15:39:31Z

Thanks @cjeria

I am working on these changes now. One note regarding the bug with resolving your ens name.

It seems that there was some sort of error when you registered it. See the difference between https://etherscan.io/address/christianjeria.eth and https://etherscan.io/address/dinodan.eth for example, or between https://manager.ens.domains/name/christianjeria.eth and https://manager.ens.domains/name/dinodan.eth

If you go to https://etherscan.io/enslookup and enter your ens name, it looks like the address was previously in a different registrar but has not successfully registered in the new registrar.

For us in MetaMask, we should show an error when this happens instead of a 0x...0x address. I will make that change as well.

metamaskbot · 2019-07-31T16:55:19Z

Builds ready [b989bd3]: chrome, firefox, edge, opera

…x; allow display of addressbook name in settings header

…the ens address.

…try if it exists

…omponent)

…hecksummed address

…nd proper internal navigation

metamaskbot · 2019-07-31T18:12:40Z

Builds ready [8ebdf9f]: chrome, firefox, edge, opera

whymarrh

LGTM, I've reviewed, QA'd, and reviewed again ❤️

metamaskbot · 2019-07-31T19:46:46Z

Builds ready [e8baae8]: chrome, firefox, edge, opera

danjm requested review from Gudahtt and whymarrh as code owners July 25, 2019 14:58

danjm force-pushed the address-book-send-plus-contact-list branch from 2e24129 to fbb2fa9 Compare July 25, 2019 15:06

danjm force-pushed the address-book-send branch from 414ed87 to 4578c87 Compare July 25, 2019 15:08

danjm changed the base branch from address-book-send to develop July 25, 2019 15:56

whymarrh suggested changes Jul 25, 2019

View reviewed changes

danjm mentioned this pull request Jul 26, 2019

Design QA [S17] MetaMask/Design#120

Open

3 tasks

danjm force-pushed the address-book-send-plus-contact-list branch from 305e96a to 88aa542 Compare July 26, 2019 00:29

danjm force-pushed the address-book-send-plus-contact-list branch from 1fa7533 to 4d8295f Compare July 26, 2019 02:47

danjm force-pushed the address-book-send-plus-contact-list branch from 4d8295f to a8439a8 Compare July 26, 2019 03:41

danjm force-pushed the address-book-send-plus-contact-list branch from a8439a8 to 1b6430d Compare July 26, 2019 12:53

danjm mentioned this pull request Jul 26, 2019

3box integration #6876

Closed

chikeichan added 8 commits July 31, 2019 15:19

Style Send Header

775210b

Move Send to-row to send view and restyle

1a6a547

Add "Recents" group to select recipient view

3d187c3

Rename SendToRow to AddRecipient

882a698

Basic UI and Layout

2cae0bf

New ENSInput component

b42bd35

wip - fuzzy search for input

e71a968

small refactor

e044048

danjm added 20 commits July 31, 2019 15:22

Refactor settings component to use a container for connection to redu…

057cb32

…x; allow display of addressbook name in settings header

Decouple ens-input.component from send context

63a356d

Add ens resolution to add contact screen in settings

9439730

Switching networks when an ens address is shown on send form removes …

3461311

…the ens address.

Resolve send screen search for ensAddress to matching address book en…

13a548c

…try if it exists

Show resolved ens icon and address if exists (settings: add-contact.c…

5d1483a

…omponent)

Make the displayed and copied address in view-contact.component the c…

623ea6f

…hecksummed address

Default alias state prop in AddToAddressBookModal to empty string

5ca4ad0

Use keyCode to detect enter key in AddToAddressBookModal

1dc96f0

Ensure add-contact component properly updates after QR code detection

006bb78

Fix display of all recents after clicking 'Load More' in contact list

26a217b

Fix send screen contact searching after network switching

bd145f7

Code cleanup related to address book changes

4f8bbf2

Update unit tests for address book changes

df7f29d

Update ENS name not found on network message

7327cb8

Add ens registration error message

e4dd6dc

Cancel on edit mode takes user back to view screen

6de7221

Adds support for memo to settings contact list view and edit screens

9438adf

Modify designs of edit and view contact in popup environment

0552b13

Update settings content list UX to show split columns in fullscreen a…

8ebdf9f

…nd proper internal navigation

whymarrh force-pushed the address-book-send-plus-contact-list branch from 9ed68fe to 8ebdf9f Compare July 31, 2019 17:53

Correct background address book API usages in UI

e8baae8

whymarrh approved these changes Jul 31, 2019

View reviewed changes

danjm merged commit e9c7df2 into develop Jul 31, 2019

whymarrh mentioned this pull request Jul 31, 2019

Address book send #6681

Closed

8 tasks

jennypollack mentioned this pull request Aug 15, 2019

Addressbook should be Network-specific #6789

Closed

danjm deleted the address-book-send-plus-contact-list branch October 2, 2019 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address book send plus contact list #6914

Address book send plus contact list #6914

danjm commented Jul 25, 2019 •

edited

Loading

metamaskbot commented Jul 25, 2019

whymarrh left a comment

whymarrh Jul 25, 2019

whymarrh Jul 25, 2019

whymarrh Jul 25, 2019

whymarrh Jul 25, 2019

whymarrh Jul 25, 2019

whymarrh Jul 25, 2019

whymarrh Jul 25, 2019

whymarrh Jul 25, 2019

whymarrh Jul 25, 2019

whymarrh commented Jul 25, 2019

danjm commented Jul 26, 2019

metamaskbot commented Jul 26, 2019

danjm commented Jul 26, 2019

metamaskbot commented Jul 26, 2019

metamaskbot commented Jul 26, 2019

cjeria commented Jul 30, 2019 •

edited

Loading

danjm commented Jul 30, 2019

metamaskbot commented Jul 31, 2019

metamaskbot commented Jul 31, 2019

whymarrh left a comment

metamaskbot commented Jul 31, 2019

Address book send plus contact list #6914

Address book send plus contact list #6914

Conversation

danjm commented Jul 25, 2019 • edited Loading

metamaskbot commented Jul 25, 2019

whymarrh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whymarrh commented Jul 25, 2019

danjm commented Jul 26, 2019

metamaskbot commented Jul 26, 2019

danjm commented Jul 26, 2019

metamaskbot commented Jul 26, 2019

metamaskbot commented Jul 26, 2019

cjeria commented Jul 30, 2019 • edited Loading

ENS functionality

ENS error message copy

ENS address not displaying (bug)

Contact list

Cancelling out of Edit Mode

Memo

Current implementation

Fullscreen mode

danjm commented Jul 30, 2019

metamaskbot commented Jul 31, 2019

metamaskbot commented Jul 31, 2019

whymarrh left a comment

Choose a reason for hiding this comment

metamaskbot commented Jul 31, 2019

danjm commented Jul 25, 2019 •

edited

Loading

cjeria commented Jul 30, 2019 •

edited

Loading