Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/dexpro/orderbook/bignum alignment #2465

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

smk762
Copy link
Collaborator

@smk762 smk762 commented Aug 12, 2024

Closes https://github.com/KomodoPlatform/komodo-wallet-desktop/issues/2427

To test:

  • login and activate MAZA
  • go to pro view, and select KMD/MAZA for orderbook
  • confirm no misalignment

@cipig
Copy link
Member

cipig commented Aug 13, 2024

it now looks like this
image
pay attention to the bids... there is one for 30k MAZA and one for 300k MAZA... they look the same (because they have a different number of decimals)
the ask for 9k and the one for 99k also look the same
i fear we can't solve the problem by using different number of decimals for different entries in same view... we need to cut down the decimals for all entries to the same number or we need to make the column wider

@AndrewDelaney
Copy link
Collaborator

Everything does look very nice and neatly aligned. The only issue I have is with the column headings, and the amount of space left unutilised. I agree with @cipig, the columns can be made wider. I have created a proposed idea for an improved layout that may remedy the issue @cipig raised.

Before After
OrderBookAlignment OrderBookAlignment(Proposal)

This layout will help show more of the number and make it easier to compare. I know we round to 8 decimal places, but perhaps we should round down to the last non-zero digit within the 8 decimal places? Then right aligning all the columns to make comparisons easier.
e.g.
70.540000000085 -> 70.54
45.00000000 -> 45.0
97.6800430000091 -> 97.680043

Just an idea, though this is a lot more work than simple alignment, and we'd have to look at changing it across the board.

@cipig
Copy link
Member

cipig commented Aug 15, 2024

This layout will help show more of the number and make it easier to compare. I know we round to 8 decimal places, but perhaps we should round down to the last non-zero digit within the 8 decimal places?

the values in the columns should be right-aligned, not left-aligned, so you can see right away which value is bigger... and you can only do that if all values have the same number of decimals, if they differ it does not work... and now they differ, see
image
one is 30k, the other 300k, but they "look" the same if you don't start counting decimals

@ShantanuSharma9873
Copy link
Collaborator

@cipig
Copy link
Member

cipig commented Aug 19, 2024

if we need to save space, we could reduce the number of decimals for "Quantity" and "Total" to 6, in general, for all coins/orderbooks... for BTC (the coin with highest value in USD) the min. volume in a swap is anyway 0.00777000, so orders with lower volumes can't be posted... besides that, default dust for all coins is 546 sats (0.00000546)

atm this change makes it worser, since it "damages" orderbooks that previously looked fine, like DOGE/KMD:
image

@smk762 smk762 changed the base branch from dev to dev-kdf August 22, 2024 13:13
@cipig
Copy link
Member

cipig commented Aug 22, 2024

just realized that we already had it to 6 decimals for "Quantity" and "Total"
this is how DOGE/KMD orderbook looks like without this PR:
image
and MAZA/KMD:
image
the big orders seen in https://github.com/KomodoPlatform/komodo-wallet-desktop/issues/2427#issue-2233330136 are gone in the meantime, so the problem is only with big numbers, then it gets unaligned... i fear the only way to solve it is to make tab wider

@smk762 smk762 changed the base branch from dev-kdf to dev August 23, 2024 03:55
Copy link
Member

@cipig cipig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much better, thanks
here is one example with big values... BTTC-BEP20/KMD
image
only problem i see is that it uses different numbers of decimals... the most expensive ask has 4 decimals in "Quantity", the others have 3
but such cases are now pretty rare

@smk762 smk762 merged commit 7a6e41d into dev Aug 26, 2024
6 checks passed
@ca333 ca333 deleted the fix/dexpro/orderbook/bignum-alignment branch October 23, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: Quantity of bids is not right-aligned any more
4 participants