Page MenuHomePhabricator

[Cashtab][Ailas] Remove aliasToRegister and aliasLength state vars
ClosedPublic

Authored by emack on Oct 11 2023, 00:02.

Details

Summary

The aliasToRegister state var can be accessed directly via formData.aliasName, hence removal.

As a direct impact, aliasLength is also redundant and can be accessed via formData.aliasName.length.

Test Plan

npm test
set alias config flag to true
npm start
type in an alias for registration and observe correct parsing of length
type in an alias with invalid length and observe input error
shorten the previous invalid lengthed alias into a valid length and observe the input error disappears
submit the alias for registration and observe correct parsing of name and price in confirmation dialog and subsequent name display in the tx history tab after broadcast
submit an already registered alias and ensure preparePreviewModal picks this up with an error
(don't worry about the non-rendering of pending and registered aliases as these are being implemented in D14611

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emack requested review of this revision.Oct 11 2023, 00:02
bytesofman added inline comments.
cashtab/src/components/Alias/Alias.js
521 ↗(On Diff #42627)

note that this only works because, for now, aliases must be alphanumeric

might as well use the available getAliasByteSize function to make this robust against future utf8 support

This revision now requires changes to proceed.Oct 11 2023, 11:51
emack marked an inline comment as done.

Using getAliasByteSize in place of formData.aliasName.length.

Fabien requested changes to this revision.Oct 12 2023, 08:19
Fabien added a subscriber: Fabien.
Fabien added inline comments.
cashtab/src/components/Alias/Alias.js
400 ↗(On Diff #42629)

Unrelated: this whole logic can be removed

521 ↗(On Diff #42629)

Can you store that in a variable so getAliasByteSize() is only called once ?

This revision now requires changes to proceed.Oct 12 2023, 08:19
emack requested review of this revision.Oct 12 2023, 09:55
emack marked an inline comment as done.
emack added inline comments.
cashtab/src/components/Alias/Alias.js
400 ↗(On Diff #42629)

Will remove in a separate diff via T3316

521 ↗(On Diff #42629)

The starting point for this diff was a aliasLength variable which stores this but the previous feedback was to remove it in favour of direct accessing the formData. @bytesofman - given it's used in more than one place I'm leaning back to restoring the aliasLength state var.

cashtab/src/components/Alias/Alias.js
521 ↗(On Diff #42629)

It doesn't need to be a state variable, just a local variable for readability and reducing the computation time

emack marked an inline comment as done.

Added aliasLength as a local var

Updated snapshot @generated tag

This revision is now accepted and ready to land.Oct 16 2023, 08:29