[ENG-3594] feature: Send Max STX #767

jordankzf · 2024-01-23T04:11:56Z

🔘 PR Type

What kind of change does this PR introduce?

Notes:

Success screen restyle not part of this PR.
Confirmation screen restyle not part of this PR.
Estimated fees seem weird.

📜 Background

Provide a brief explanation of why this pull request is needed. Include the problem you are solving or the functionality you are adding. Reference any related issues.

Issue Link: #[issue_number]
Context Link (if applicable):

🔄 Changes

Enumerate the changes made in this pull request, detailing what has been modified, added, or removed. Include technical details and implications if necessary.

Impact:

Explain the broader impact of these changes.
How it improves performance, fixes bugs, adds functionality, etc.

🖼 Screenshot / 📹 Video

Include screenshots or a video demonstrating the changes. This is especially helpful for UI changes.

✅ Review checklist

Please ensure the following are true before merging:

Code Style is consistent with the project guidelines.
Code is readable and well-commented.
No unnecessary or debugging code has been added.
Security considerations have been taken into account.
The change has been manually tested and works as expected.
Breaking changes and their impacts have been considered and documented.
Code does not introduce new technical debt or issues.

…-consolidation-logic-tx

Co-authored-by: Jordan K. <[email protected]>

…-consolidation-logic-tx

and default to currentColor in spinner

…-extension into jordankzf/send-max-stx

src/app/screens/sendStx/index.tsx

teebszet · 2024-02-26T05:52:33Z

wrong font on the amount input fiat rate here, and the up-down-arrows icon looks too small compared to the figma. also check the padding around the fiat rate?

teebszet · 2024-02-26T06:57:05Z

src/app/ui-library/inputFeedback.tsx


 const colors: Record<FeedbackVariant, string> = {
  info: Theme.colors.white_200,
  danger: Theme.colors.danger_light,
+  plain: Theme.colors.white_200,


this should be 'explicative' to match zeroheight naming, and white_400

figma:

teebszet · 2024-02-26T07:00:09Z

src/app/ui-library/inputFeedback.tsx


 const colors: Record<FeedbackVariant, string> = {
  info: Theme.colors.white_200,
  danger: Theme.colors.danger_light,
+  plain: Theme.colors.white_200,
+  plainIndented: Theme.colors.white_200,


nit: maybe this should be a indentedNoIcon?: boolean prop?
so that any of the variants can replace the icon with an indent

alternatively, you could use

noIcon?: boolean

and add a margin-left where you're using it for bns names, because we don't want to bloat the ui-library with random exceptions that may not be used much in other places

teebszet · 2024-02-26T07:06:56Z

src/app/ui-library/button.tsx

@@ -6,10 +6,13 @@ type ButtonVariant = 'primary' | 'secondary' | 'tertiary' | 'danger';

 const StyledButton = styled.button`
  width: 100%;
+  min-height: 42.5px;


question: is this something that is needed for all buttons and their variations? in zeroheight it's height: 44px.

if it's only for an edge case usage, consider applying this min-height where you need it rather than enforcing it here

teebszet · 2024-02-26T07:52:51Z

src/app/screens/sendStx/steps/Step2SelectAmount.tsx

+      new BigNumber(stxBtcRate),
+      new BigNumber(btcFiatRate),
+    )
+      .toNumber()


nit: is toNumber() necessary here?

teebszet · 2024-02-26T08:02:01Z

src/app/screens/sendStx/steps/Step2SelectAmount.tsx

+              fiatUnit={fiatCurrency}
+              getFeeForFeeRate={getFeeForFeeRate}
+              feeRates={fees}
+              feeRateLimits={{ min: 0.000001, max: feeMultipliers?.thresholdHighStacksFee }}


is this min/max logic stated somewhere in requirements?

No, it's not documented. It's the same logic used for the current send stx implementation.

teebszet · 2024-02-26T09:36:12Z

src/app/screens/sendStx/steps/Step2SelectAmount.tsx

+      const [low, medium, high] = await estimateTransaction(
+        unsignedTx.payload,
+        undefined,
+        selectedNetwork,
+      );
+      setFees({
+        low: Number(microstacksToStx(new BigNumber(low.fee)).toFixed(2)),
+        medium: Number(microstacksToStx(new BigNumber(medium.fee)).toFixed(2)),
+        high: Number(microstacksToStx(new BigNumber(high.fee)).toFixed(2)),
+      });
+      if (!fee)
+        setFeeRate(Number(microstacksToStx(new BigNumber(medium.fee)).toFixed(2)).toString());


need to clarify with the requirements on what this logic should be. it looks to me that the notion requirements point to this section: https://www.notion.so/xverseapp/Speed-up-Stacks-transaction-Improve-fee-estimation-4455d27f8bdc4ebfbd94a16d7f18fe12?pvs=4#9509f0afd87d40eba297b357aa28763c

but there's no capping logic being applied here

teebszet · 2024-02-26T09:41:59Z

@jordankzf I think the main points here are:

need to clarify and fix what the stx fee estimates should be (low, med, high)
need to fix the minor UI issues
need to think more carefully about how you're extending/duplicating the btc send max components, especially around the ui-components/selectFeeRate, as @victorkirov commented on. currently, the code to extend selectFeeRate for send stx feels a bit clunky

I think in order to keep this feature moving, let's focus on 1. and 2. to be ready for testing, and 3. can be a follow up task to include refactoring how best to share code between the various send flows, (including btc, stx, and other fungible token protocols)

github-actions · 2024-03-01T06:20:48Z

Test with build here: https://github.com/secretkeylabs/xverse-web-extension/actions/runs/8107403960

victorkirov added 21 commits January 15, 2024 09:52

Add btc send screen skeleton

d6df19b

Implement MAX

4ccd4dd

extract amount selector to component

76c9f31

Add max dust filtered callout

63f2bb4

Fix minor styling

2ea224a

Add fee display

5207c5d

Implement fee selection

ed664f4

Add insufficient funds on fee selector

de83299

add insufficient funds message on custom fee selector

b20e4b6

Add insufficient funds gate

002dc58

fix some todos

b7e4ded

Add min and max fee rate

4d1b558

fix container styling

e2b0b1a

Merge branch 'develop' into victor/eng-3548-ext-send-btc-integrate-tx…

5f79616

…-consolidation-logic-tx

fix styling

f304895

Fix fee summary on confirm page

a14b8d4

fix some component interactions

225f1bd

fix a few minor bugs

23dd157

fix rbf typing

75fa52d

Merge branch 'develop' into victor/eng-3548-ext-send-btc-integrate-tx…

b92ad64

…-consolidation-logic-tx

Close window if ledger

cf3cc1f

jordankzf marked this pull request as draft January 23, 2024 04:12

victorkirov and others added 4 commits January 23, 2024 08:58

Update src/app/ui-library/input.tsx

aecf750

Co-authored-by: Jordan K. <[email protected]>

Update src/app/ui-library/input.tsx

38223d4

Co-authored-by: Jordan K. <[email protected]>

Add address editable field

0d0409c

Merge branch 'develop' into victor/eng-3548-ext-send-btc-integrate-tx…

d2925dd

…-consolidation-logic-tx

jordankzf force-pushed the jordankzf/send-max-stx branch from 3b265b1 to c5ab910 Compare January 28, 2024 05:53

jordankzf changed the base branch from develop to victor/eng-3548-ext-send-btc-integrate-tx-consolidation-logic-tx January 28, 2024 05:55

teebszet added 2 commits January 31, 2024 14:28

chore: package-lock exact version

bbd6829

refactor: use classnames for variants in ui-library button

5a56cd8

and default to currentColor in spinner

jordankzf added 2 commits February 14, 2024 10:32

Remove comments

cf11c64

avoid calling setState twice

b79f6b5

Base automatically changed from victor/eng-3548-ext-send-btc-integrate-tx-consolidation-logic-tx to develop February 15, 2024 09:55

jordankzf added 2 commits February 17, 2024 11:05

Merge branch 'develop' of https://github.com/secretkeylabs/xverse-web…

88d39b5

…-extension into jordankzf/send-max-stx

Rename btcRecipientSelector

441bf77

abdulhaseeb4239 reviewed Feb 20, 2024

View reviewed changes

src/app/screens/sendStx/index.tsx Show resolved Hide resolved

Address Vic feedback

7425a8c

jordankzf requested review from victorkirov and abdulhaseeb4239 February 20, 2024 09:25

teebszet removed their request for review February 21, 2024 03:05

teebszet added the Release 0.30.0 label Feb 23, 2024

teebszet reviewed Feb 26, 2024

View reviewed changes

teebszet added Release 0.31.0 and removed Release 0.30.0 labels Feb 26, 2024

abdulhaseeb4239 assigned jordankzf Feb 27, 2024

abdulhaseeb4239 removed their request for review February 28, 2024 04:36

abdulhaseeb4239 added 4 commits February 28, 2024 15:39

address PR comments

1b30168

fix: change variant name according to zeroheight

77346dd

fix: use the correct variant for memo

d24473c

feat: apply stx fee cap logic

51ec511

fix: setting recipient address

2b6e22f

teebszet closed this Apr 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENG-3594] feature: Send Max STX #767

[ENG-3594] feature: Send Max STX #767

jordankzf commented Jan 23, 2024 •

edited

Loading

teebszet commented Feb 26, 2024 •

edited

Loading

teebszet Feb 26, 2024

teebszet Feb 26, 2024 •

edited

Loading

teebszet Feb 26, 2024 •

edited

Loading

teebszet Feb 26, 2024

teebszet Feb 26, 2024 •

edited

Loading

teebszet Feb 26, 2024

jordankzf Feb 27, 2024

teebszet Feb 26, 2024

teebszet commented Feb 26, 2024 •

edited

Loading

github-actions bot commented Mar 1, 2024

[ENG-3594] feature: Send Max STX #767

[ENG-3594] feature: Send Max STX #767

Conversation

jordankzf commented Jan 23, 2024 • edited Loading

🔘 PR Type

📜 Background

🔄 Changes

🖼 Screenshot / 📹 Video

✅ Review checklist

teebszet commented Feb 26, 2024 • edited Loading

teebszet Feb 26, 2024

Choose a reason for hiding this comment

teebszet Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

teebszet Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

teebszet Feb 26, 2024

Choose a reason for hiding this comment

teebszet Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

teebszet Feb 26, 2024

Choose a reason for hiding this comment

jordankzf Feb 27, 2024

Choose a reason for hiding this comment

teebszet Feb 26, 2024

Choose a reason for hiding this comment

teebszet commented Feb 26, 2024 • edited Loading

github-actions bot commented Mar 1, 2024

jordankzf commented Jan 23, 2024 •

edited

Loading

teebszet commented Feb 26, 2024 •

edited

Loading

teebszet Feb 26, 2024 •

edited

Loading

teebszet Feb 26, 2024 •

edited

Loading

teebszet Feb 26, 2024 •

edited

Loading

teebszet commented Feb 26, 2024 •

edited

Loading