feat: create StackTokenPair #38

berteotti · 2023-08-02T11:52:14Z

Creates StackTokenPair section. Formatted balance isn't consistent throughout the code base.

Diogomartf · 2023-08-02T12:24:28Z

While testing got Application error.

details

Using Brave Version 1.56.14 Chromium: on a Macbook air

How to reproduce:

connected wallet
select wxdai
error screen

Note: Selecting other tokens seems fine.

Looking at the console it looks like:

RangeError: maximumSignificantDigits value is out of range.
` at Number.toLocaleString

Diogomartf · 2023-08-02T12:29:50Z

Would be nice that the size of decimals was the same as balance on bottom right.

like this 8,0994 instead of 8,099426770481235208

Diogomartf · 2023-08-02T12:31:44Z

I think we should clean the input when changing tokens,

what do you think?

Diogomartf · 2023-08-02T12:33:28Z

packages/app/components/stackbox/Stackbox.tsx

@@ -70,6 +86,14 @@ export const Stackbox = () => {
    setEndDateTime(new Date(endDateByFrequency[frequency]));
  }, [frequency]);

+  const formattedBalance = (balanceData: NonNullable<typeof balance>) =>


this function could be extracted to utils/token file

I think we should uniformize all this formates in the app. And add this formatting to a specific file. If you agree will create a task to do this. But we can keep this function inside Stackbox for this PR.

Agree on standardizing formats throughout the app but I agree with Diogo to abstract this function into a utils file, because, in my experience, these functions tend to grow and change their signature as the app needs to grow. At some point, I'm sure we will abstract this function, sooner or later.

I don't mind handling that abstraction in a different PR as long as we handle it, either here or in another task.

Created this issue to tackle this in our app https://linear.app/swaprdev/issue/STK-104/format-token-amounts

berteotti · 2023-08-03T19:53:30Z

Would be nice that the size of decimals was the same as balance on bottom right.

like this 8,0994 instead of 8,099426770481235208

I think it's important to let the user define the granularity of what he is spending. If we hide tokens he can't know if he is using max balance or just a portion of it.

berteotti · 2023-08-03T19:55:23Z

I think we should clean the input when changing tokens,

what do you think?

I think we should let it for now. If the user inputs a custom value and it changes token, I think we probably wants to keep that value. Also thought about this and the behaviour on other apps is to keep the value.

cloudflare-workers-and-pages · 2023-08-04T08:45:56Z

Deploying with Cloudflare Pages

Latest commit:	`f5406ef`
Status:	✅ Deploy successful!
Preview URL:	https://36fdc472.stackly-ui.pages.dev
Branch Preview URL:	https://feat-stk-32-create-stacktoke.stackly-ui.pages.dev

View logs

Diogomartf · 2023-08-04T12:42:37Z

Would be nice that the size of decimals was the same as balance on bottom right.
like this 8,0994 instead of 8,099426770481235208

I think it's important to let the user define the granularity of what he is spending. If we hide tokens he can't know if he is using max balance or just a portion of it.

Just use the same as balance (as we also choose to not show all the decimals of the balance) and use only that portion not the whole decimals. That's just too many numbers.

We let the user go to that kind of granularity if he wants.

Diogomartf · 2023-08-04T12:42:57Z

While testing got Application error.

details

Using Brave Version 1.56.14 Chromium: on a Macbook air

How to reproduce:

connected wallet

select wxdai

error screen

Note: Selecting other tokens seems fine.
Looking at the console it looks like:
RangeError: maximumSignificantDigits value is out of range. ` at Number.toLocaleString

still not fixed

berteotti · 2023-08-04T12:51:16Z

Would be nice that the size of decimals was the same as balance on bottom right.
like this 8,0994 instead of 8,099426770481235208

I think it's important to let the user define the granularity of what he is spending. If we hide tokens he can't know if he is using max balance or just a portion of it.

Just use the same as balance (as we also choose to not show all the decimals of the balance) and use only that portion not the whole decimals. That's just too many numbers.

We let the user go to that kind of granularity if he wants.

How can we let the user define the granularity if he doesn't know the entire balance? If the user clicks Max we have to assume he wants to spend the entire token amount, not just an approximation of that value. What do you think?

Diogomartf · 2023-08-04T14:08:04Z

Would be nice that the size of decimals was the same as balance on bottom right.
like this 8,0994 instead of 8,099426770481235208

I think it's important to let the user define the granularity of what he is spending. If we hide tokens he can't know if he is using max balance or just a portion of it.

Just use the same as balance (as we also choose to not show all the decimals of the balance) and use only that portion not the whole decimals. That's just too many numbers.
We let the user go to that kind of granularity if he wants.

How can we let the user define the granularity if he doesn't know the entire balance? If the user clicks Max we have to assume he wants to spend the entire token amount, not just an approximation of that value. What do you think?

I would prefer to not be shown so many decimals. It's so small that only adds noise.

But I'll let you decide on it now. We can change it in the future if we want (add/remove decimals).

Diogomartf · 2023-08-07T10:15:22Z

packages/app/components/stackbox/Stackbox.tsx

+  const formattedBalance = (balanceData: NonNullable<typeof balance>) => {
+    const SIGNIFICANT_DIGITS = 5;
+
+    /* parseFloat as some number size limitations so this is a temporary solution */


Suggested change

/* parseFloat as some number size limitations so this is a temporary solution */

/* parseFloat has some number size limitations so this is a temporary solution */

what are the limitations?

Description is clearer now.

Diogomartf reviewed Aug 2, 2023

View reviewed changes

ElRodrigote previously approved these changes Aug 3, 2023

View reviewed changes

berteotti dismissed ElRodrigote’s stale review via e366931 August 4, 2023 08:45

berteotti force-pushed the feat/stk-32-create-stacktokenpair branch from 647e1a6 to e366931 Compare August 4, 2023 08:45

berteotti force-pushed the feat/stk-32-create-stacktokenpair branch from e366931 to 1658799 Compare August 7, 2023 09:21

berteotti requested review from Diogomartf and ElRodrigote August 7, 2023 09:27

Diogomartf reviewed Aug 7, 2023

View reviewed changes

Diogomartf previously approved these changes Aug 7, 2023

View reviewed changes

feat: create StackTokenPair

f5406ef

berteotti dismissed Diogomartf’s stale review via f5406ef August 7, 2023 10:34

berteotti force-pushed the feat/stk-32-create-stacktokenpair branch from 1658799 to f5406ef Compare August 7, 2023 10:34

Diogomartf approved these changes Aug 7, 2023

View reviewed changes

berteotti merged commit 72cf6f3 into develop Aug 7, 2023
2 checks passed

berteotti deleted the feat/stk-32-create-stacktokenpair branch August 7, 2023 10:47

berteotti added a commit that referenced this pull request Oct 19, 2023

feat: create StackTokenPair (#38)

86900c7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create StackTokenPair #38

feat: create StackTokenPair #38

berteotti commented Aug 2, 2023

Diogomartf commented Aug 2, 2023 •

edited

Loading

Diogomartf commented Aug 2, 2023

Diogomartf commented Aug 2, 2023

Diogomartf Aug 2, 2023 •

edited

Loading

berteotti Aug 3, 2023

ElRodrigote Aug 3, 2023

berteotti Aug 4, 2023

berteotti commented Aug 3, 2023

berteotti commented Aug 3, 2023

cloudflare-workers-and-pages bot commented Aug 4, 2023 •

edited

Loading

Diogomartf commented Aug 4, 2023

Diogomartf commented Aug 4, 2023 •

edited

Loading

berteotti commented Aug 4, 2023

Diogomartf commented Aug 4, 2023

Diogomartf Aug 7, 2023 •

edited

Loading

berteotti Aug 7, 2023

	/* parseFloat as some number size limitations so this is a temporary solution */
	/* parseFloat has some number size limitations so this is a temporary solution */

feat: create StackTokenPair #38

feat: create StackTokenPair #38

Conversation

berteotti commented Aug 2, 2023

Diogomartf commented Aug 2, 2023 • edited Loading

Diogomartf commented Aug 2, 2023

Diogomartf commented Aug 2, 2023

Diogomartf Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

berteotti Aug 3, 2023

Choose a reason for hiding this comment

ElRodrigote Aug 3, 2023

Choose a reason for hiding this comment

berteotti Aug 4, 2023

Choose a reason for hiding this comment

berteotti commented Aug 3, 2023

berteotti commented Aug 3, 2023

cloudflare-workers-and-pages bot commented Aug 4, 2023 • edited Loading

Deploying with Cloudflare Pages

Diogomartf commented Aug 4, 2023

Diogomartf commented Aug 4, 2023 • edited Loading

berteotti commented Aug 4, 2023

Diogomartf commented Aug 4, 2023

Diogomartf Aug 7, 2023 • edited Loading

Choose a reason for hiding this comment

berteotti Aug 7, 2023

Choose a reason for hiding this comment

Diogomartf commented Aug 2, 2023 •

edited

Loading

Diogomartf Aug 2, 2023 •

edited

Loading

cloudflare-workers-and-pages bot commented Aug 4, 2023 •

edited

Loading

Diogomartf commented Aug 4, 2023 •

edited

Loading

Diogomartf Aug 7, 2023 •

edited

Loading