Page MenuHomePhabricator

[Cashtab][Alias] Status check upon alias registration input pause
ClosedPublic

Authored by emack on Dec 1 2023, 07:09.

Details

Summary

As per UAT feedback in T3363, this adds an event listener to the aliasName input field so that when the user stops typing for at least 1 second it triggers an API call to check the alias availability. If the alias is unavailable, display error message and disable buttons.

image.png (402×670 px, 29 KB)

This does not include pending registrations because as per spec users should still be able to submit them should they wish to compete for the registration result.

Test Plan
  • Navigate to the Alias component with no input into the Alias field and observe no keystroke event listener triggered via the dev console
  • Input an already registered alias (e.g. watermelon) and ensure event listener does not trigger the API call whilst typing however it triggers the API call once you've stopped typing for 1 second, which results in a "This alias is already owned by ecash:qz..., please try another alias" UI error message. Ensure buttons are disabled upon this.
  • Input an unregistered alias and ensure the buttons are enabled, with no errors displayed.
  • Simulate an alias-server outage by malforming the 'alias' endpoint, and observe the "Error retrieving alias details" UI error when you pause an alias input for 1 second and disabling of buttons.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
timedAliasCheck
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 25804
Build 51188: Build Diffcashtab-tests
Build 51187: arc lint + arc unit

Event Timeline

emack requested review of this revision.Dec 1 2023, 07:09

That's smart, but what is the point of the check button then ? Can it be removed ?

emack planned changes to this revision.Dec 1 2023, 09:33

That's smart, but what is the point of the check button then ? Can it be removed ?

Yea the Check button can be removed as this is a more elegant solution. I'll update this diff to also render a UI message when the alias is available for consistency.
I'll deprecate the check button in a separate diff.

emack requested review of this revision.Dec 1 2023, 10:55

That's smart, but what is the point of the check button then ? Can it be removed ?

Yea the Check button can be removed as this is a more elegant solution. I'll update this diff to also render a UI message when the alias is available for consistency.
I'll deprecate the check button in a separate diff.

After reviewing, there is no need for a specific 'alias is available' message upon this API call because unless it is unavailable, the alias length and registration price will be displayed which is intuitive enough to the user that it is ok to proceed.

image.png (181×560 px, 17 KB)

As above, will deprecate Check Alias button in another diff.

bytesofman requested changes to this revision.Dec 1 2023, 15:22

This does not include pending registrations because as per spec users should still be able to submit them should they wish to compete for the registration result.

Still useful to know there are pending registrations though. The user still gets a popup msg / warning in this case on trying to register?

cashtab/src/components/Alias/Alias.js
188 ↗(On Diff #43337)

let's give 1000 some kind of variable assignment so it isn't a magic number

const KEY_UP_TIMEOUT_MS = 1000

This revision now requires changes to proceed.Dec 1 2023, 15:22
emack marked an inline comment as done.

Still useful to know there are pending registrations though. The user still gets a popup msg / warning in this case on trying to register?

yup the confirmation modal via preparePreviewModal already provides the pending warnings which is the next step in this workflow anyway, hence I don't think we need to duplicate it here.

Moved key up timeout to aliasSettings config.

bytesofman requested changes to this revision.Dec 4 2023, 17:06
bytesofman added inline comments.
cashtab/src/components/Alias/Alias.js
157
161

won't timeout always be undefined here?

Please implement appropriate way to cleanup setTimeout in a useEffect, e.g.

https://stackoverflow.com/questions/53090432/react-hooks-right-way-to-clear-timeouts-and-intervals

This revision now requires changes to proceed.Dec 4 2023, 17:06
emack marked 2 inline comments as done.

Moved the clearTimeout call to the end of useEffect() when the component unmounts

This revision is now accepted and ready to land.Dec 5 2023, 17:53