Page MenuHomePhabricator
Differential D17484

[Cashtab] Improve form entry and validation for listing tokens on agora
ClosedPublic
Actions

Authored by bytesofman on Tue, Jan 7, 13:36.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCdbd1f19eb7b9: [Cashtab] Improve form entry and validation for listing tokens on agora
Summary

Right now, users enter price last. This means we validate the price field for the minimum buy being at least dust. This is confusing to users because it looks like they need to charge more per token in order to list, when in reality they need to offer more of the token.

A better workflow here is to enter the price 2nd. That way it's:

  1. How many tokens are you selling?
  2. What's the price per token?
  3. What's the min buy?

In this diff, we automatically set the min buy after the user enters (1) and (2). The min buy must be such that the min buy of the offer is at least 546 satoshis.

A price is valid as long as it is at least 1 nanosat per token sat. We do not want to encourage users to try a higher price if they are attached to a low min qty.

Test Plan

npm test

This diff is deployed at https://cashtab-local-dev.netlify.app/

Diff Detail

Repository
rABC Bitcoin ABC
Branch
improve-error-msg
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 31955
Build 63402: Build Diffcashtab-tests
Build 63401: arc lint + arc unit

Event Timeline

bytesofman created this revision.Tue, Jan 7, 13:36
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Jan 7, 13:36
Harbormaster completed remote builds in B31955: Diff 52012.Tue, Jan 7, 13:41
teamcity added a comment.Tue, Jan 7, 13:41
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

  npm audit fix

Run `npm audit` for details.

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 23s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.4 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/validation/index.ts
  Line 12:5:  'decimalizeTokenAmount' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 52022.Tue, Jan 7, 15:15

start updating the tests, not finished.

Harbormaster completed remote builds in B31962: Diff 52022.Tue, Jan 7, 15:15
teamcity added a comment.Tue, Jan 7, 15:36
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

  npm audit fix

Run `npm audit` for details.

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 1s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 23s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.6 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/validation/index.ts
  Line 12:5:  'decimalizeTokenAmount' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 52023.Tue, Jan 7, 19:28

Update tests

Harbormaster completed remote builds in B31963: Diff 52023.Tue, Jan 7, 19:33
teamcity added a comment.Tue, Jan 7, 19:33
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

  npm audit fix

Run `npm audit` for details.

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 25s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.6 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/validation/index.ts
  Line 12:5:  'decimalizeTokenAmount' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 52024.Tue, Jan 7, 19:45

validation tests

Harbormaster completed remote builds in B31964: Diff 52024.Tue, Jan 7, 19:50
teamcity added a comment.Tue, Jan 7, 19:50
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

  npm audit fix

Run `npm audit` for details.

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 25s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.6 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/validation/index.ts
  Line 12:5:  'decimalizeTokenAmount' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 52025.Tue, Jan 7, 19:51

document new validation function, remove debug logs, remove unused css components

bytesofman edited the summary of this revision. (Show Details)Tue, Jan 7, 19:53
bytesofman edited the test plan for this revision. (Show Details)
Harbormaster completed remote builds in B31965: Diff 52025.Tue, Jan 7, 19:56
teamcity added a comment.Tue, Jan 7, 19:56
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

  npm audit fix

Run `npm audit` for details.

> ecash-lib@1.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 25s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.10.6 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/validation/index.ts
  Line 12:5:  'decimalizeTokenAmount' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 52027.Tue, Jan 7, 20:05
bytesofman edited the summary of this revision. (Show Details)

remove unused param from test, ts build lint

Harbormaster completed remote builds in B31967: Diff 52027.Tue, Jan 7, 20:11
bytesofman published this revision for review.Tue, Jan 7, 20:12
bytesofman added inline comments.
cashtab/src/components/Common/Inputs.tsx
611 ↗(On Diff #52025)

We allow the slider to be disabled. In this way, we disable min qty input until the user has input price

cashtab/src/components/Etokens/Token/index.tsx
26 ↗(On Diff #52025)

we add a new validation function that matches the model of other validation functions. before this diff, we used a combination of checks and functions to get min buy validation. now just one function.

622 ↗(On Diff #52025)

logic change in this useEffect

Before this diff:
We validated the min buy qty when the min buy qty changed and when the total offer qty changed, to make sure that we were not creating an unacceptable offer (min buy above total offered amount).

After this diff
We do the same as before
We also automatically update the min buy field to the lowest min buy that would create a valid offer, based on the user's price
Because this qty may be invalid (it may exceed the total offered amount), we validate it in that case, too

946 ↗(On Diff #52025)

simplified validation errors with one function. can see before we had multiple checks. the new function is necessary because now we are validating min buy qty as the "last thing" whereas before we did this with price.

cashtab/src/components/Etokens/__tests__/TokenActions.test.js
294 ↗(On Diff #52025)

we update existing tests to confirm new behavior, including automated updating of the min buy qty and automated validation checks of this update

cashtab/src/validation/__tests__/index.test.js
571 ↗(On Diff #52025)

unit tests for new validation function

cashtab/src/validation/index.ts
1224 ↗(On Diff #52025)

new validation function and doc

Fabien added a subscriber: Fabien.Wed, Jan 8, 08:31
Fabien added inline comments.
cashtab/src/components/Common/Inputs.tsx
611 ↗(On Diff #52025)

This should be a comment in the code

bytesofman marked an inline comment as done.Wed, Jan 8, 12:00
bytesofman added inline comments.
cashtab/src/components/Common/Inputs.tsx
611 ↗(On Diff #52025)

In this case I think it is handled adequately by typescript itself and the tests

For example, can ctrl+f on this diff for // Min qty input is disabled before we enter offered qty -- we confirm in the unit tests that the slider is disabled when we expect it to be, then enabled when we expect it to be.

typescript itself will let the dev know that disabled is an available optional prop -- and that tslin does not break here (and other tests do not fail) shows that this change does not break existing use of the slider without the disabled prop.

emack requested changes to this revision.Thu, Jan 9, 06:22
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Etokens/Token/index.tsx
634 ↗(On Diff #52027)
660 ↗(On Diff #52027)

If the user enters 0 as min qty, they'll get a message telling them to enter a value greater than 0.

image.png (85×1 px, 6 KB)

If they follow that message and enter in 1, and if 1 is below the min token buy qty then they'll get a different error telling them it needs to be above a different min qty, which may cause confusion.
image.png (74×870 px, 8 KB)

Following the logic flow, this message is hard coded in isValidTokenSendOrBurnAmount. I would suggest updating it to react differently when amount === '0'.

cashtab/src/validation/fixtures/vectors.js
2923 ↗(On Diff #52027)

update as per above

This revision now requires changes to proceed.Thu, Jan 9, 06:22
bytesofman marked 4 inline comments as done.Thu, Jan 9, 06:32
bytesofman added inline comments.
cashtab/src/components/Etokens/Token/index.tsx
660 ↗(On Diff #52027)

this could be improved but I don't think we do it here. this diff is already pretty complicated, and this is existing behavior.

So we won't change a function this diff does not touch because we noticed in this diff that it could be improved.

cashtab/src/validation/fixtures/vectors.js
2923 ↗(On Diff #52027)

this test just happens to pick this error -- it's passed from isValidTokenSendOrBurnAmount which this diff doesn't touch.

This test just confirms we can get error msgs from this function that is called in the new function.

bytesofman updated this revision to Diff 52058.Thu, Jan 9, 06:33
bytesofman marked 2 inline comments as done.

improve comment readability, rebase

Harbormaster completed remote builds in B31987: Diff 52058.Thu, Jan 9, 06:38
emack accepted this revision.Thu, Jan 9, 10:19
This revision is now accepted and ready to land.Thu, Jan 9, 10:19
Closed by commit rABCdbd1f19eb7b9: [Cashtab] Improve form entry and validation for listing tokens on agora (authored by bytesofman). · Explain WhyThu, Jan 9, 12:14
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCdbd1f19eb7b9: [Cashtab] Improve form entry and validation for listing tokens on agora.

Revision Contents
Changeset List

PathSize
cashtab/
src/
components/
Common/
4 lines
Etokens/
Token/
138 lines
validation/
68 lines

Diff 52012

View Options

cashtab/src/components/Common/Inputs.tsx

Loading...
View Options

cashtab/src/components/Etokens/Token/index.tsx

Loading...
View Options

cashtab/src/validation/index.ts

Loading...