LedgerHQ / app-bitcoin-new

Modern Bitcoin Application based on PSBT and Descriptors
Apache License 2.0
93 stars 69 forks source link

Improve wording of 'Sign transaction' UX flows. #252

Closed bigspider closed 5 months ago

bigspider commented 5 months ago

This changes the wording during transaction signatures:

Before: txflow-old After (but without removing the "Confirm transaction" step): txflow-new

Final: txflow-new-2

Closes: #248

codecov-commenter commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.85%. Comparing base (0a905b1) to head (eb66957).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #252 +/- ## ======================================== Coverage 84.85% 84.85% ======================================== Files 17 17 Lines 2192 2192 ======================================== Hits 1860 1860 Misses 332 332 ``` | [Flag](https://app.codecov.io/gh/LedgerHQ/app-bitcoin-new/pull/252/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LedgerHQ) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/LedgerHQ/app-bitcoin-new/pull/252/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LedgerHQ) | `84.85% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LedgerHQ#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

blucet-ledger commented 5 months ago

@bigspider the screen preceding the fee display shows "Confirm transaction", which seems too generic to be useful for users. What do you think of:

bigspider commented 5 months ago

@bigspider the screen preceding the fee display shows "Confirm transaction", which seems too generic to be useful for users. What do you think of:

  • making it more informative and accurate, eg. "Review fees";
  • or removing it altogether?

Not opposed to idea; I'd prefer removing it, it does indeed seem redundant.

bigspider commented 5 months ago

If there are no outputs (all the money spent is going to a change address), since version 2.1.3 we show a simplified UX:

txflow-selftransfer

In this case it's worth keeping the "Confirm self-transfer" screen, even if we remove it for normal transactions.

bigspider commented 5 months ago

As per the discussion above, deleted the 'Confirm transaction' screen. New flow:

txflow-new-2

kloaec commented 5 months ago

(edit: maybe i should have commented on the Issue and not the PR, happy to move it if you prefer.)

Great changes, thanks for that.

For consistency, I would recommend that you also use the same rewording on other flows, such as the "register descriptor" flow, which uses the "approve" term instead of "continue", for intermediary steps.

Another flow that I use every day: spending from Liana. First step is "spend from known wallet"... "approve". This approve should be a "continue?" or even "review details". (because "continue" spending isn't really clear either!)

The rest of the PR is great, let's remove the "approve" where they are just a "next"/"continue" button.

bigspider commented 5 months ago

For consistency, I would recommend that you also use the same rewording on other flows, such as the "register descriptor" flow, which uses the "approve" term instead of "continue", for intermediary steps.

Yes, that's a good idea. I prefer to keep it for a future PR because I'm hoping to explore not having a 'Continue' step at all: you just keep going right until the end, and then you have a single "Approve" step at the end. In principle, that could be done for other flows as well, but it might be quite tricky because of how the current UX framework works. I'll open a separate issue for the wallet registration flow.

Another flow that I use every day: spending from Liana. First step is "spend from known wallet"... "approve". This approve should be a "continue?" or even "review details". (because "continue" spending isn't really clear either!)

It's already changed with 'Continue' in this PR, for example: txflow-from-wallet

sonarcloud[bot] commented 5 months ago

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
12.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud