Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABCe43d81ba0711: [Cashtab][Alias] Upgrade Alias UI

Summary

T3216

Upgrading the Alias UI to be a bit more prod-worthy and in anticipation of the need to separate Registered and Pending aliases. This diff introduces dropdowns and tags whereby within the Registered Aliases dropdown it renders the registered aliases into Tags, which can be clicked on to copy the alias to the clipboard.
This is also an efficient use of the UI real estate to avoid users having to scroll endlessly if they have registered a lot of aliases.

Tag color is aligned to branding guidelines.

Test Plan

npm test
set alias flag to true in Ticker.js
npm start and navigate to the Alias page
expand the Registered Aliases dropdown and verify the correct alphabetical rendering of aliases attached to this wallet and on hover, cursor recognizes this as a link
click on any of the registered aliases and ensure the full alias is copied to clipboard
register a 21 byte alias, wait for confirmation and ensure no UI funkiness with wrapping over the dropdown border
create a new wallet and ensure the dropdown displays N/A
trigger an alias-server outage and observe error rendered within the Registered Aliases dropdown

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

emack created this revision.Aug 2 2023, 03:32

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 2 2023, 03:32

emack requested review of this revision.Aug 2 2023, 03:32

Harbormaster completed remote builds in B24654: Diff 41674.Aug 2 2023, 03:35

emack added inline comments.Aug 2 2023, 03:36

cashtab/src/components/Alias/Alias.js
119 ↗	(On Diff #41674)	this unrelated change was enforced by arc lint

emack added inline comments.Aug 2 2023, 03:50

cashtab/src/components/Alias/Alias.js
110 ↗	(On Diff #41674)	For some reason this diff causes alias.test.js to fall over because `wallet.Path1899` is undefined when the `Without wallet defined` test gets up to const thisAddress = convertToEcashPrefix(wallet.Path1899.cashAddress); But this diff tdoesn't ouch anything related to this. Master branch is also fine with this too. I've temporarily added this check on page load, which probably should be there in the first place but I'm still narrowing down why this particular diff is causing this.

emack added inline comments.Aug 2 2023, 03:54

cashtab/src/components/Alias/Alias.js
110 ↗	(On Diff #41674)	Attaching the correct screenshot

emack edited the summary of this revision. (Show Details)Aug 2 2023, 03:57

Can the alias be sorted ? That would make it much easier to find one.

bytesofman added inline comments.Aug 2 2023, 18:31

cashtab/src/components/Alias/Alias.js
119 ↗	(On Diff #41674)	is this based on the latest master? or mb an IDE change overriding lint? if i run `arc lint --everything` on latest master, i don't get any changes to Cashtab

Aliases now sorted in ascending order, tested by registering a new alias and it slotted in correctly in alphabetical order.
I thought about adding a toggle to switch between ascending and descending but there's not enough whitespace without it feeling super cramped.
Also re-linted Alias.js to revert that weird newline.

Harbormaster completed remote builds in B24667: Diff 41692.Aug 3 2023, 11:45

emack edited the test plan for this revision. (Show Details)Aug 3 2023, 11:59

looks like this is not against the latest master, i'm not able to arc patch. Does this diff depend on another diff that is unlisted in the summary?

cashtab/src/components/Alias/Alias.js
555 ↗	(On Diff #41692)	already using `alias.alias` as key for `<CopyToClipboard>` component
557 ↗	(On Diff #41692)	? what's going on here
565 ↗	(On Diff #41692)

This revision now requires changes to proceed.Aug 3 2023, 12:31

Responding to inline feedback

cashtab/src/components/Alias/Alias.js
555 ↗	(On Diff #41692)	Appended with Tag string to differentiate from clipboard key
557 ↗	(On Diff #41692)	The <a href> means upon mouse hover it's recognized as clickable with the mouse cursor turning to the clicking finger. The {void 0} part means it does nothing upon mouse click.
565 ↗	(On Diff #41692)	Updated

Harbormaster completed remote builds in B24671: Diff 41697.Aug 3 2023, 13:56

bytesofman requested changes to this revision.Aug 3 2023, 17:09

bytesofman added inline comments.

cashtab/src/components/Alias/Alias.js
557 ↗	(On Diff #41692)	effective but I don't think good practice If we want the mouse pointer to change, we can do that with styled components This should be a styled component and not a link-that-isn't-actually-a-link Might be possible to style the parent `<Tag>` , but if not, can create `<AliasLabel>`