Page MenuHomePhabricator

[Cashtab] Clean up conditions for disabling Send button
ClosedPublic

Authored by bytesofman on Jan 9 2024, 23:28.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABC8a8e8f4fce61: [Cashtab] Clean up conditions for disabling Send button
Summary

T3385

Many validation checks on Send screen -- together with desire to not show validation errors when user first loads the screen -- have made this set of conditions complex.

Store in single variable and add comments.

Disable send on page initial load state, which is not valid. Because isNaN('') === false (TIL), add condition for this check (prevents case of address being entered, value not yet entered -- button is enabled and you can still get a nonsense modal).

Test Plan

npm test

npm start, navigate to send screen. Button should be disabled. Enter valid address. Button still disabled. Enter valid amount. Button enabled.

Enter valid multisend, valid msg input.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
disable-send-if-forms-blank
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 26248
Build 52068: Build Diffcashtab-tests
Build 52067: arc lint + arc unit

Event Timeline

bytesofman published this revision for review.Jan 9 2024, 23:30
bytesofman edited the summary of this revision. (Show Details)
bytesofman edited the test plan for this revision. (Show Details)
emack requested changes to this revision.Jan 10 2024, 00:56
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Send/Send.js
641 ↗(On Diff #44008)

Should this be in a standalone function in validations.js so it can come with unit tests? Unless this is a specific reason related to how the component is rendered before function s here are triggered hence needing to declare it here?

This revision now requires changes to proceed.Jan 10 2024, 00:56

Separate function and add unit tests

bytesofman added inline comments.
cashtab/src/components/Send/Send.js
641 ↗(On Diff #44008)

good point

bytesofman marked an inline comment as done.

header to 2024 for new file

This revision is now accepted and ready to land.Jan 11 2024, 00:41