Page MenuHomePhabricator

[Cashtab] Patch overflow from error string in send one to many input
ClosedPublic

Authored by kieran709 on Aug 18 2022, 16:28.

Details

Summary

related to T2610. When inputting an incorrect eCash address in the DestinationAddressMulti input, a long string caused the error message to extend beyond the boundaries of the app. Container div added to avoid this issue.

Test Plan

cd web/cashtab && npm start
navigate to send screen
in the advanced dropdown, toggle the multiple recipients switch
input a long string that is not a valid XEC address
observe that the error message does not extend beyond the boundary of the app

Diff Detail

Repository
rABC Bitcoin ABC
Branch
patch-error-overflow-one-to-many
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 19895
Build 39497: Build Diffcashtab-tests
Build 39496: arc lint + arc unit

Event Timeline

bytesofman added inline comments.
web/cashtab/src/components/Send/Send.js
941 ↗(On Diff #34708)

Is there a reason this fix could not be implemented by modifying the existing <DestinationAddressMulti> component?

Preference is to modify existing component and not use a wrapper. Wrapper is acceptable if className is required to patch on <DestinationAddressMulti>

This revision now requires changes to proceed.Aug 18 2022, 21:11
web/cashtab/src/components/Send/Send.js
941 ↗(On Diff #34708)

I didn't think I could which was why I initially used the container div, however on closer inspection there was actually a typo in EnhancedInputs.js which was making it impossible to implement the changes from there. Pushing now with the typo fixed to show that it works, but if fixing the typo requires its own diff I can certainly make a task/diff for that.

Responding to review feedback

image.png (559×583 px, 55 KB)

Maybe not related to this diff but what's going on with this undefined?

To recreate, copy paste a blockhash into the field, like 00000000000000000f8513a596bb077a8e54cb02b89ba3d1d91b43cb08afad05

This revision now requires changes to proceed.Aug 18 2022, 22:52

Undefined is showing because there is no ,amount following the address. I can create a task / diff to fix it so that does not show.

The undefined issue is addressed in T2615 / D11877.

The undefined issue is addressed in T2615 / D11877.

D11877 is green. Please rebase this diff after landing D11877

Successfully rebased to master.

This revision is now accepted and ready to land.Aug 30 2022, 17:06