Added AddressBook feature by asif-finimble · Pull Request #2414 · AlphaWallet/alpha-wallet-android

asif-finimble · 2022-02-17T12:33:32Z

@JamesSmartCell I have implemented AddressBook feature and added it to InputAddress component. Here are the details regarding the implementation. Please let me know if I need to change/update any thing.

AddressBook Activity

2 modes : Normal and Contact selection
Normal mode
- shows list of contacts and allows to add more contacts by clicking + button in toolbar
- opens edit address screen on clicking any item.
- allows swipe to delete
Contact selection mode: Used for selecting a contact from address book
- OnClicking an item, it is set as activity result for the calling activity to receive the contact.
- does not allow swipe to delete

AddEditAddress Activity

Modes: Add & Edit

Add
- if ens name is entered in Address, if it resolves, address will be set in address box and the ens name will be auto filled in Name box. Also ens name will be recorded for the entered address.
- on clicking save, if data us valid, it is added to address book and activity closes.
- Validations:
  - Address and name should not be empty
  - Address should be valid. Utils.isAddressValid()
  - Address & Name doesnt already exist in exisiting contacts.
Edit
- Address input is disabled. address cannot be edited. Only name is editable
- on clicking save, if data is valid, the contact name is updated.
- Validation rules
  - Name is not empty
  - Old name and new name are different
  - new name is not assigned to other contact.
  - Address is valid.

Input Address Component

showAddressBook attribute allows to show AddressBook text beside the paste text.
On clicking, AddressBook text, AddressBook Activity is opened for contact selection.
The host activity of input address component should handle onActivityResult to get the data returned by AddressBookActivity

TransactionDetailActivity

If the sender and receiver address are present in address book, their name will be shown instead of address. On Clicking copy button, the address will be copied.
If address is not present in AddressBook, AddressBook icon is shown instead of copy icon. Onclicking it, AddEditAddressActivity is opened with the address auto filled.

Activity Adapter

passing AddressBookInteract to activity adapter to resolve the to & from address names if present in address book.
Not posiible to resolve as address is returned in the form of 0x42c...95cs

…ddressBook.

JamesSmartCell · 2022-02-19T01:25:04Z

@asif-finimble this is a very large PR for a small feature. Can we prune it a little to get the code change down a little.

I can see a few wins in the duplicated resources, there are a few strings which are already translated.

Also, the adapter sliding has already been written, no need for a new class to do the same thing. Please see TokensAdapter which uses the token sliding (if that's what it's for - I might have mis-read it).

Let's work together after I get back from the conference to work out how to reduce the code size; it looks like a good implementation but I'm pretty sure we can re-use a lot of existing components rather than writing new ones that duplicate function.

asif-finimble · 2022-02-19T04:20:16Z

OK. I went through the code and the sliding classes are used as inner classes so they are tightly coupled. It wont work with AddressBook.

JamesSmartCell · 2022-02-24T04:05:41Z

OK. I went through the code and the sliding classes are used as inner classes so they are tightly coupled. It wont work with AddressBook.

Ok, but then that slider should be moved out of the class rather than duplicating code. Can probably just move the slider inner class out verbatim as we know it's reliable (unless it accesses private vars, in which case it'll need a new callback interface).

Adding AddressBook should not need to touch 58 files. It should only operate on InputAddress, possibly a BottomSheetDialog. I don't think it's been fully designed yet. Let me get back to you on this one. I agree with your implementation it should be a Realm item rather than SharedPrefs.

asif-finimble · 2022-02-25T11:15:13Z

The sliding class is only extending ItemTouchHelper.SimpleCallback and overriding the required methods for drawing the content to show for sliding and the implementation of each of them is tightly coupled and specific to that class. Even if we generalize the sliding class most of the code is different and we cannot really reuse the class. Let me what we should do for this.

As for the AddressBook, I found that the AddressBook feature was not implemented. So first I implemented the AddressBook feature by referring the zeplin designs and after that integrated it with InputAddress. So I had to create the UI, repository , interact, etc for AddressBook and that's why there are changes to 58 files. Let me know what should I do further.

asif-finimble · 2022-03-01T10:51:18Z

@JamesSmartCell I resolved the conflicts.

seabornlee · 2022-03-21T04:24:58Z

+    /**
+     * This is associated ETH name of #walletAddress
+     */
+    private String ethName;


Do you mean ens name?

seabornlee · 2022-03-21T04:31:57Z

+        }
+        catch (Exception e)
+        {
+            e.printStackTrace();


Should log it.
https://stackoverflow.com/questions/3855187/is-it-a-bad-idea-to-use-printstacktrace-for-caugt-exceptions

Updated logging with Timber.

seabornlee · 2022-03-21T04:33:04Z

+                    .equalTo("walletAddress", oldContact.getWalletAddress())
+                    .findFirst();
+            if (realmContact != null) {
+//                realmContact.setName(newContact.getName());


The comment can be removed.

seabornlee · 2022-03-21T04:47:32Z

+        } else {
+            mode = ADDRESS_OPERATION_MODE.EDIT;
+            setTitle(getString(R.string.title_edit_address));
+            contactToEdit = getIntent().getParcelableExtra(C.EXTRA_CONTACT);


Duplicated.

seabornlee · 2022-03-21T04:48:49Z

+            String name = inputViewName.getText().toString().trim();
+            boolean isDataValid = true;
+            if (address.isEmpty()) {
+                inputViewAddress.setError("Address should not be empty");


seabornlee · 2022-03-21T04:56:29Z

+            data.add(position, item);
+            notifyItemInserted(position);
+        } catch(Exception e) {
+        }


asif-finimble · 2022-03-22T12:01:32Z

@seabornlee, code updated with changes.

seabornlee · 2022-03-22T12:20:12Z

@seabornlee, code updated with changes.

LGTM, thanks a lot!

Added AddressBook feature and updated InputAddress component to use A…

81e2c4e

…ddressBook.

asif-finimble requested a review from JamesSmartCell February 17, 2022 12:33

asif-finimble added 2 commits March 1, 2022 11:24

Reuse strings.

a7305ef

Resolved conflicts and updated code.

c2479e5

seabornlee reviewed Mar 21, 2022

View reviewed changes

minor refactor and fixes

92aa497

JamesSmartCell force-pushed the master branch from e967340 to 32f6049 Compare October 30, 2022 23:57

JamesSmartCell force-pushed the master branch 2 times, most recently from a1cfb4f to 8acd3f7 Compare December 7, 2022 02:09

JamesSmartCell force-pushed the master branch from 0e760a3 to 2f1161a Compare March 26, 2024 01:58

Conversation

asif-finimble commented Feb 17, 2022

AddressBook Activity

AddEditAddress Activity

Input Address Component

TransactionDetailActivity

Activity Adapter

Uh oh!

JamesSmartCell commented Feb 19, 2022

Uh oh!

asif-finimble commented Feb 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JamesSmartCell commented Feb 24, 2022

Uh oh!

asif-finimble commented Feb 25, 2022

Uh oh!

asif-finimble commented Mar 1, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asif-finimble commented Mar 22, 2022

Uh oh!

seabornlee commented Mar 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

asif-finimble commented Feb 19, 2022 •

edited

Loading