feat:(label) Improved address label user experience #1978

otkstudio · 2022-11-17T22:04:26Z

New label UX for easier labelling and cleaner UX. Targeting this issue #1922

Screen.Recording.2022-11-17.at.2.03.05.PM.mov

netlify · 2022-11-17T22:04:31Z

✅ Deploy Preview for specter-desktop-docs canceled.

Name	Link
🔨 Latest commit	`b673168`
🔍 Latest deploy log	https://app.netlify.com/sites/specter-desktop-docs/deploys/639cba0e685cf300092c66c7

k9ert · 2022-11-21T15:27:06Z

Awesome! Looks really good! LGTM!

moneymanolis · 2022-11-23T09:58:06Z

We are trying to support Firefox and Chrome/Chromium, so my tests are with both:

Chrome:
Works really well. Just the positioning of the service logos could be improved:
Update: Fixed!
Under addresses:

Not fixed yet
Under transactions - history:

Updat: Fixed!
Firefox:
Doesn't really work, no cursor, can't confirm, only cancel, can't input anything:
https://user-images.githubusercontent.com/70536101/203537013-ff484053-a025-4f2a-9bfe-69c6470736fe.mov

moneymanolis

Not working on Firefox yet.

moneymanolis · 2022-11-23T11:49:01Z

src/cryptoadvance/specter/templates/includes/address-data.html

@@ -40,6 +40,9 @@
            border: 1px solid var(--main-color);
            background: var(--cmap-border);
        }
+        tr {
+            vertical-align: center;


Suggested change

vertical-align: center;

vertical-align: middle;

not sure whether it changes anything to have this - in any case center is an invalid property for vertical-align

moneymanolis · 2022-11-23T13:35:33Z

I wanted to add proper backend validation / error handling at wallets api endpoint to ensure consistency between front end changes and the backend. Following the code I can see that we use rpc calls (setlabel and listlabels) in addresslist.py which confused me since I thought we didn't use rpc calls in this respect anymore. @stepansnigirev @k9ert - could you help clarify?

…ecter-desktop into improved-address-labels

moneymanolis · 2022-11-23T17:14:56Z

src/cryptoadvance/specter/templates/includes/address-label.html

        .label {
            word-break: break-all;
            background: none;
            color: #fff;
            font-size: 1em;
            max-width: 80%;
            outline: none;
+            margin-top: 1px;
+            margin-left: 2px;
+            white-space: nowrap;


This causes that the addresses here are not fully displayed:

We could keep the nowrap and just shorten the addresses like we do in the addresses overview:

…ecter-desktop into improved-address-labels

moneymanolis · 2022-11-23T20:24:37Z

Short video on the changes in d5a0c16 (not seen in the video is that if the label is empty it can still be saved, makes kind of sense since we have logos and perhaps someone just wants to have the logo):
https://user-images.githubusercontent.com/70536101/203639739-36984c83-6a97-460b-b4ce-1ca120217d50.mov

moneymanolis · 2022-11-23T20:28:51Z

@otkstdio please have a look at my changes. If okay, could you do the two remaining tasks:

Fix address display in transactions - history
Fix service logo positioning in transactions - history
(both issues were "introduced" with this PR)

moneymanolis · 2022-11-23T20:29:31Z

@relativisticelectron could you do a quick test?

otkstudio · 2022-11-24T19:34:49Z

@moneymanolis All fixed 👍

relativisticelectron · 2022-11-25T12:58:38Z

I like the "click to edit" UX!

For long labels and narrow screen, the other columns are not visible any more.

clicking into an empty field of the table should end the edit mode

moneymanolis · 2022-11-25T14:34:22Z

@otkstdio can you have a look at electron's points?

otkstudio · 2022-11-27T19:35:02Z

Hey @relativisticelectron!

I think we need a solution to the label being too long in another PR (one I'm working on). This is certainly a problem but it needs to be fixed in the context of the table not being able to handle all sized inputs, not just the label. I think we'll need the ability to scroll horizontally in tables with extended data points. Also, what's the use case of having a label of an address the same as the address? I'd hope users wouldn't actually do this as labels are effectively human readable descriptors to reflect the not-so-human-readable address, not the address itself.

When it comes to clicking on other elements, I suspect this would show the user intent of cancelling the editing process and thus not updating the label. Would you agree?

relativisticelectron · 2022-11-29T07:48:41Z

On master long labels are wrapped to multiple lines, such that all columns stay visible. I think this is the desirable behavior.

When it comes to clicking on other elements, I suspect this would show the user intent of cancelling the editing process and thus not updating the label. Would you agree?

Yes.

moneymanolis · 2022-11-29T08:09:20Z

@otkstdio I'd also say let's keep the wrapping of the addresses for now and change that once you came up with a more general solution that you were mentioning above:

This is certainly a problem but it needs to be fixed in the context of the table not being able to handle all sized inputs, not just the label.

moneymanolis · 2022-11-30T19:00:18Z

Some more issues:

The border here looks a bit random, should either be unified or dropped:

This PR deletes the copy functionality of an address in the address list via the explorer link. This at first looks like no issue but if you open up tx info for example, there is now no way anymore to copy an address there. Possible solution: we could just add the address here alongside the label wherever needed - perhaps add copy / past functionality, but selection of the address would probably be enough here:

As for the long label with multisig address, I'd suggest to change the default label of the address by sth. like "External address", see screenshot above? @relativisticelectron what do you think?

Add a copy icon similar to the QR code icon when hovering over the address in the address list to "compensate" for the deleted explorer link.

moneymanolis · 2022-12-14T13:12:36Z

As for

long labels are not wrapping

I know. @otkstdio will take care of that in his first or second round of CSS revamp.

The console message is fine / intended.
Thanks for the hint on the click propagation, will do that!

moneymanolis · 2022-12-14T17:24:51Z

@relativisticelectron could you please test again?

relativisticelectron · 2022-12-14T18:33:54Z

@relativisticelectron could you please test again?

Sorry, but it still doesn't work. Moving with left-right arrow keys works, but not with the mouse.

moneymanolis · 2022-12-15T08:23:57Z

@relativisticelectron but this is intended / normal in the selection all that we use. Please test by removing the whole selection and then type sth. and by using the arrows and add sth. for example.
In particular, does the spacebar work as intended?

relativisticelectron · 2022-12-15T08:30:32Z

@relativisticelectron but this is intended / normal in the selection all that we use.

I expect an editable field to work not only with the keyboard but also with the mouse. The wallet-name edit function works as I would expect an editable field to work:

Please test by removing the whole selection and then type sth. and by using the arrows and add sth. for example. In particular, does the spacebar work as intended?

Yes, spacebar works as intended.

moneymanolis · 2022-12-15T08:57:46Z

The relevant lines are (in the edit.onclick() in address-label.html)

    let selection = window.getSelection();
    selection.selectAllChildren(this.label);

The idea behind this is, that you usually want to replace the default label in the addresses (sth. like Address #5) completely and the above lines make this easier. Perhaps we can check whether this is the first click on the label (enabling the editing and thus use select all) and then the next clicks are not selecting all anymore.

UPDATE: Implemented in b4f1abf - @relativisticelectron - perhaps one last check?

relativisticelectron · 2022-12-15T20:07:19Z

It seems clicking on the side of the update button fails to update it. (Center clicking works) The console shows Clicking somewhere else on the screen. Canceling editing.
The "Select All" at edit start is nice. Good work

Other points:

Thanks for the hint on the click propagation, will do that!

For long labels and narrow screen, the other columns are not visible any more.

moneymanolis · 2022-12-16T09:37:58Z

It seems clicking on the side of the update button fails to update it. (Center clicking works) The console shows Clicking somewhere else on the screen. Canceling editing.

I see, cancelEditing was called first (via the global event listener which ensures the "clicking somewhere else cancels the editing").

From GPT:
"Event listeners are executed in the order in which they are registered. If you register the global event listener first and then the event listener for the update button, the global event listener will be executed first and then the event listener for the update button will be executed."

I am using stopPropagation now on all click event handler in the component so the global one is not even receiving the event and it also makes checking where the click originated from in the global listener obsolete.

@relativisticelectron that should have fixed your last point. Thanks a lot for the rigorous testing!

moneymanolis · 2022-12-16T16:02:38Z

@relativisticelectron can I merge?

moneymanolis · 2022-12-16T18:57:50Z

I am merging this now and will directly open an issue with @relativisticelectron input to deal better with long labels.

feat:(label) improved address label ux

ce0d1cf

otkstudio requested a review from moneymanolis November 17, 2022 22:04

moneymanolis requested changes Nov 23, 2022

View reviewed changes

moneymanolis reviewed Nov 23, 2022

View reviewed changes

Merge branch 'master' into improved-address-labels

33ecc24

moneymanolis and others added 3 commits November 23, 2022 17:49

service icon alignment

cd0eb5f

Merge branch 'improved-address-labels' of github.com:cryptoadvance/sp…

c74e155

…ecter-desktop into improved-address-labels

Merge branch 'master' into improved-address-labels

2b77b1e

moneymanolis reviewed Nov 23, 2022

View reviewed changes

moneymanolis added 2 commits November 23, 2022 21:11

make it work on firefox

d5a0c16

Merge branch 'improved-address-labels' of github.com:cryptoadvance/sp…

4488033

…ecter-desktop into improved-address-labels

otkstudio added 3 commits November 24, 2022 11:24

fix:(label) fix spacing of the service logo on tx table

05171c4

fix:(label) fix sizing of service icon within label

4649727

fix:(label) fix table not scaling to label width

6cd2f2e

Merge branch 'master' into improved-address-labels

c323949

add / change titles

e689e8c

firefox fixes

fbd7d95

moneymanolis added 2 commits December 14, 2022 19:20

prevent bubbling

1607e03

check whether there is an address-label for txlist

0f48852

moneymanolis added 2 commits December 15, 2022 14:40

select all only at edit start

b4f1abf

revert copy address button

809cec7

moneymanolis requested a review from k9ert December 15, 2022 15:38

moneymanolis mentioned this pull request Dec 15, 2022

Optimise position of Swan logo for withdrawal addresses #1980

Closed

moneymanolis linked an issue Dec 15, 2022 that may be closed by this pull request

Optimise position of Swan logo for withdrawal addresses #1980

Closed

moneymanolis added 3 commits December 15, 2022 20:27

add editable attribute

d6e08db

disable editing for receive page

acb4829

change cursor to default in non-editable mode

fd0eb4d

moneymanolis added 2 commits December 16, 2022 10:38

disable bubbling for update & cancel btns

eb61708

no need to check for specific event targets anymore

ce20e87

moneymanolis approved these changes Dec 16, 2022

View reviewed changes

moneymanolis mentioned this pull request Dec 16, 2022

Chore: Cypress test for address labeling #2021

Merged

k9ert approved these changes Dec 16, 2022

View reviewed changes

Merge branch 'master' into improved-address-labels

b673168

moneymanolis merged commit ab0245c into master Dec 16, 2022

moneymanolis mentioned this pull request Dec 16, 2022

Dealing with long labels #2022

Open

k9ert deleted the improved-address-labels branch February 3, 2023 10:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat:(label) Improved address label user experience #1978

feat:(label) Improved address label user experience #1978

otkstudio commented Nov 17, 2022 •

edited

Loading

netlify bot commented Nov 17, 2022 •

edited

Loading

k9ert commented Nov 21, 2022

moneymanolis commented Nov 23, 2022 •

edited

Loading

moneymanolis left a comment

moneymanolis Nov 23, 2022 •

edited

Loading

moneymanolis commented Nov 23, 2022

moneymanolis Nov 23, 2022

moneymanolis commented Nov 23, 2022

moneymanolis commented Nov 23, 2022 •

edited by otkstudio

Loading

moneymanolis commented Nov 23, 2022

otkstudio commented Nov 24, 2022

relativisticelectron commented Nov 25, 2022 •

edited by moneymanolis

Loading

moneymanolis commented Nov 25, 2022

otkstudio commented Nov 27, 2022

relativisticelectron commented Nov 29, 2022

moneymanolis commented Nov 29, 2022

moneymanolis commented Nov 30, 2022 •

edited

Loading

moneymanolis commented Dec 14, 2022 •

edited

Loading

moneymanolis commented Dec 14, 2022 •

edited

Loading

relativisticelectron commented Dec 14, 2022 •

edited

Loading

moneymanolis commented Dec 15, 2022

relativisticelectron commented Dec 15, 2022 •

edited

Loading

moneymanolis commented Dec 15, 2022 •

edited

Loading

relativisticelectron commented Dec 15, 2022 •

edited

Loading

moneymanolis commented Dec 16, 2022 •

edited

Loading

moneymanolis commented Dec 16, 2022

moneymanolis commented Dec 16, 2022

feat:(label) Improved address label user experience #1978

feat:(label) Improved address label user experience #1978

Conversation

otkstudio commented Nov 17, 2022 • edited Loading

netlify bot commented Nov 17, 2022 • edited Loading

✅ Deploy Preview for specter-desktop-docs canceled.

k9ert commented Nov 21, 2022

moneymanolis commented Nov 23, 2022 • edited Loading

moneymanolis left a comment

Choose a reason for hiding this comment

moneymanolis Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

moneymanolis commented Nov 23, 2022

moneymanolis Nov 23, 2022

Choose a reason for hiding this comment

moneymanolis commented Nov 23, 2022

moneymanolis commented Nov 23, 2022 • edited by otkstudio Loading

moneymanolis commented Nov 23, 2022

otkstudio commented Nov 24, 2022

relativisticelectron commented Nov 25, 2022 • edited by moneymanolis Loading

moneymanolis commented Nov 25, 2022

otkstudio commented Nov 27, 2022

relativisticelectron commented Nov 29, 2022

moneymanolis commented Nov 29, 2022

moneymanolis commented Nov 30, 2022 • edited Loading

moneymanolis commented Dec 14, 2022 • edited Loading

moneymanolis commented Dec 14, 2022 • edited Loading

relativisticelectron commented Dec 14, 2022 • edited Loading

moneymanolis commented Dec 15, 2022

relativisticelectron commented Dec 15, 2022 • edited Loading

moneymanolis commented Dec 15, 2022 • edited Loading

relativisticelectron commented Dec 15, 2022 • edited Loading

moneymanolis commented Dec 16, 2022 • edited Loading

moneymanolis commented Dec 16, 2022

moneymanolis commented Dec 16, 2022

otkstudio commented Nov 17, 2022 •

edited

Loading

netlify bot commented Nov 17, 2022 •

edited

Loading

moneymanolis commented Nov 23, 2022 •

edited

Loading

moneymanolis Nov 23, 2022 •

edited

Loading

moneymanolis commented Nov 23, 2022 •

edited by otkstudio

Loading

relativisticelectron commented Nov 25, 2022 •

edited by moneymanolis

Loading

moneymanolis commented Nov 30, 2022 •

edited

Loading

moneymanolis commented Dec 14, 2022 •

edited

Loading

moneymanolis commented Dec 14, 2022 •

edited

Loading

relativisticelectron commented Dec 14, 2022 •

edited

Loading

relativisticelectron commented Dec 15, 2022 •

edited

Loading

moneymanolis commented Dec 15, 2022 •

edited

Loading

relativisticelectron commented Dec 15, 2022 •

edited

Loading

moneymanolis commented Dec 16, 2022 •

edited

Loading