Page MenuHomePhabricator
Differential D18061

[Cashtab] Allow user to receive specified amount of XEC or FIRMA on Receive page
ClosedPublic
Actions

Authored by bytesofman on Wed, May 7, 14:00.

Details

Reviewers
Fabien
emack
Group Reviewers
Restricted Project
Commits
rABC2defe2793c46: [Cashtab] Allow user to receive specified amount of XEC or FIRMA on Receive page
Summary

Cashtab send screen supports bip21 query strings. We should allow users to make their own to support point of sale use case.

This is a first pass, where we support custom XEC or FIRMA amounts.

It can be extended to support arbitary token quantities or other bip21.

new receive screen:

image.png (632×509 px, 123 KB)

image.png (632×509 px, 88 KB)

Test Plan

npm test

deployed live at test site, check at https://cashtab-local-dev.netlify.app/#/receive

Diff Detail

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

Event Timeline

bytesofman created this revision.Wed, May 7, 14:00
Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, May 7, 14:00
Harbormaster completed remote builds in B33237: Diff 53891.Wed, May 7, 14:05
bytesofman updated this revision to Diff 53895.Wed, May 7, 17:13

add tests

Harbormaster completed remote builds in B33240: Diff 53895.Wed, May 7, 17:18
bytesofman published this revision for review.Wed, May 7, 17:19
bytesofman edited the summary of this revision. (Show Details)
bytesofman edited the test plan for this revision. (Show Details)
Fabien requested changes to this revision.Thu, May 8, 10:13
Fabien added a subscriber: Fabien.

@bot preview-cashtab

cashtab/src/components/Common/Buttons.tsx
162 ↗(On Diff #53895)

? why would you want a boolean to be nullable ?

cashtab/src/components/Receive/Receive.tsx
1 ↗(On Diff #53895)

Macro whatyearisit:

This revision now requires changes to proceed.Thu, May 8, 10:13
Fabien added a comment.Thu, May 8, 10:15

@bot preview-cashtab

teamcity awarded a token.Thu, May 8, 10:15
teamcity added a comment.Thu, May 8, 10:18

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.83.66.92:41822 for the next 60 minutes.

teamcity updated a token.Thu, May 8, 10:19
teamcity added a comment.Thu, May 8, 10:22

Build Bitcoin ABC Diffs / Diff Testing (preview-cashtab) passed.
Preview is available at http://51.68.37.192:41715 for the next 60 minutes.

Fabien added a comment.Thu, May 8, 10:27

Can we have the unit in the amount input field ? IMO it avoids confusion better than the icon only, and 10 XEC is very different from 10 FIRMA

emack requested changes to this revision.Thu, May 8, 12:51
emack added a subscriber: emack.

If you input a large enough number Cashtab crashes with a WSOD and spits out a range error on console. Not a big issue but rather unsightly. The other send amount inputs throughout the app gracefully spits out a Unable to parse sendAmount as a number UI error instead of sharting.

image.png (413×640 px, 39 KB)

cashtab/src/components/Receive/QRCode.tsx
123 ↗(On Diff #53895)

what if address is neither bip21, nor token nor addressOnly types? Is the validation being done elsewhere?

bytesofman marked 3 inline comments as done.Thu, May 8, 13:54
bytesofman added inline comments.
cashtab/src/components/Common/Buttons.tsx
162 ↗(On Diff #53895)

it wouldn't really make sense for the user to specify false, tho they could. In this case it's because bool is the most effective type for the purpose of this param, but we want it to default to false. So in practice, the user --- in this case, a react dev on Cashtab -- either specifies showToast or does not. If it is unspecified, the component defaults to false below.

It's a common react convention. In this way, you can specify <CopyIconButton showToast/> and you will get this as true, no need for showToast={true}

cashtab/src/components/Receive/QRCode.tsx
123 ↗(On Diff #53895)

the only user-input field is quantity, which is validated (must be a number, only 1 decimal place, decimal place must be '.', etc)

The string itself is built programmatically in the component. So, right now, the only possible options are bip21 firma token + qty and bip21 xec address (optional qty).

It probably is possible to get isBip21 as true with bad input in the field. But in this case, it just means the user display would be bip21 query string copied to clipboard instead of the much longer actual string. imo it's ok to copy paste "bad" strings like this ... I guess we could disable it on validation error. But Cashtab and electrum already validate bip21 strings, so would see it there.

bytesofman updated this revision to Diff 53905.Thu, May 8, 13:54
bytesofman marked 2 inline comments as done.

fix year in header for Receive

Harbormaster completed remote builds in B33245: Diff 53905.Thu, May 8, 14:00
bytesofman updated this revision to Diff 53910.Thu, May 8, 22:50

support prefix and suffix for customized input field currency clarification

Harbormaster completed remote builds in B33250: Diff 53910.Thu, May 8, 22:50
bytesofman updated this revision to Diff 53911.Thu, May 8, 22:52

lint

bytesofman added inline comments.Thu, May 8, 22:56
cashtab/src/components/Receive/Receive.tsx
104–105 ↗(On Diff #53911)

note these must be manually calibrated depending on the prefix/suffix selected by the dev

it is probably possible to use calc or some other css tricks to avoid this, but in general I prefer just making the the values dev-accessible. Trying to make it automatic often leads to other complications, and I don't want to prematurely optimize something that is (for now) only used in one place.

bytesofman edited the summary of this revision. (Show Details)Thu, May 8, 22:57
Harbormaster completed remote builds in B33251: Diff 53911.Thu, May 8, 22:57
bytesofman added a comment.Thu, May 8, 23:04
In D18061#411855, @emack wrote:

If you input a large enough number Cashtab crashes with a WSOD and spits out a range error on console. Not a big issue but rather unsightly. The other send amount inputs throughout the app gracefully spits out a Unable to parse sendAmount as a number UI error instead of sharting.

I was not able to repeat this with 999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999

there is an upper limit to how much data can be encoded by a QR code, probably possible to hit it.

For now, I think it is ok to leave this to a separate diff. I don't think any users will be running into this one on accident.

tho i'll be really upset if someone was gonna send me that much FIRMA (according to grok, "ten trestrigintillion minus one"), but changed his mind after the white screen.

emack accepted this revision.Fri, May 9, 04:08
Fabien accepted this revision.Fri, May 9, 05:22
This revision is now accepted and ready to land.Fri, May 9, 05:22
Closed by commit rABC2defe2793c46: [Cashtab] Allow user to receive specified amount of XEC or FIRMA on Receive page (authored by bytesofman). · Explain WhyFri, May 9, 17:27
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABC2defe2793c46: [Cashtab] Allow user to receive specified amount of XEC or FIRMA on Receive page.

Revision Contents
Changeset List

PathSize
cashtab/
extension/
public/
2 lines
4 lines
2 lines
src/
components/
Common/
2 lines
47 lines
9 lines
Receive/
QRCode.tsx
QRCode.js
68 lines
139 lines
__tests__/
66 lines
48 lines
validation/
__tests__/
13 lines
fixtures/
172 lines
56 lines

Diff 53924

View Options

cashtab/extension/public/manifest.json

Loading...
View Options

cashtab/package-lock.json

Loading...
View Options

cashtab/package.json

Loading...
View Options

cashtab/src/components/Common/Buttons.tsx

Loading...
View Options

cashtab/src/components/Common/Inputs.tsx

Loading...
View Options

cashtab/src/components/Common/Switch.tsx

Loading...
View Options

cashtab/src/components/Receive/QRCode.js

Loading...
View Options

cashtab/src/components/Receive/QRCode.tsx

Loading...
View Options

cashtab/src/components/Receive/Receive.js

Loading...
View Options

cashtab/src/components/Receive/Receive.tsx

Loading...
View Options

cashtab/src/components/Receive/__tests__/Receive.test.js

Loading...
View Options

cashtab/src/components/Receive/styled.tsx

Loading...
View Options

cashtab/src/validation/__tests__/index.test.js

Loading...
View Options

cashtab/src/validation/fixtures/vectors.js

Loading...
View Options

cashtab/src/validation/index.ts

Loading...