Fix price format #1310

fairlighteth · 2021-08-24T17:01:01Z

Summary

Shows the right price format for a BUY or SELL order.
Addresses comment Orders Sidebar --> Show limit prices + valid to/filled on dates #1279 (comment)
Introduces a function called getPriceFormat to show the right price format for either a limit or executed price. Will need to be iterated on, once we allow switching inverse prices.

To Test

Create a buy/sell order.
Check the limit price
Should be in format INPUT amount > OUTPUT amount token for a SELL order.
Should be in format OUTPUT amount > INPUT amount token for a BUY order.

* Add bignumber, so library matches our dex-js * Add price utils * Refactor transaction summary * Change todo * Rename variable to executionPrice. * Delete comment * Remove comments and improve doc * Fix path and default value * Add datatype for api additional data (#1290) # Summary Continues #1289, Augment the order datatype so we can have the executed volumes. Adds the datatype where we can save the API information It also makes use of this data in the recent history. ## Not included Saving the actual data. Co-authored-by: biocom <michel@gnosis.pm>

github-actions · 2021-08-24T17:08:38Z

🔭 GP Swap: Gnosis Protocol v2 Swap UI

anxolin · 2021-08-24T17:27:02Z

src/custom/components/AccountDetails/Transaction/index.tsx

+      return `${price} ${kind === 'buy' ? outputAmount.currency.symbol : sellAmt.currency.symbol} per ${
+        kind === 'buy' ? sellAmt.currency.symbol : outputAmount.currency.symbol
+      }`
+    }


should this be in a more central place? maybe can be reused for other parts

The only place I can imagine is the price we use on the swap container/confirmation modal. Not sure how much of that code overlaps. You think that should be addressed in this PR or can it be a future iteration?

Reason I created a separate function here, is because otherwise there would be repetitive similar (not dry) code between the limitPrice and executionPrice.

I don't think this is right.
See my comment with the examples.

Only makes sense to have this logic if we are inverting the prices.

alfetopito · 2021-08-24T18:00:46Z

Does not look right to me.

Taking as example the top sell order:
From 27M DAI to 3k GNO, the price will be 0.0001 GNO per DAI or DAI/GNO. Not DAI per GNO.

As for the buy order:
From 3M DAI to 500 GNO, 0.001 GNO per DAI is the correct notation.

alfetopito · 2021-08-24T18:01:56Z

src/custom/components/AccountDetails/Transaction/index.tsx

+      return `${price} ${kind === 'buy' ? outputAmount.currency.symbol : sellAmt.currency.symbol} per ${
+        kind === 'buy' ? sellAmt.currency.symbol : outputAmount.currency.symbol
+      }`
+    }


I don't think this is right.
See my comment with the examples.

Only makes sense to have this logic if we are inverting the prices.

fairlighteth · 2021-08-25T09:34:06Z

@alfetopito You were right, indeed something was off. I know cleaned up the code a bit and set the key inverted to true by default. From my testing, this now matches the 'default' price display from the GP explorer.

cc @elena-zh

* Fix decimals bug * Moved additional info to BaseOrder, everything is serializable * Added additional info to order fullfilment type * Passing along additional info when fulfillig order * Storing additional info on order obj Co-authored-by: Leandro Boscariol <leandro.boscariol@gnosis.io>

…orders-panel-13

elena-zh · 2021-08-25T10:28:53Z

Hey @biocom , now the prices match GP explorer.
However, I have found 1 order where I have got a '0' price:

Frankly, I thought it might be connected to a tiny price, but when I placed an order with a lower price, I got a 'correct' value.

Here is this order
https://protocol-explorer.dev.gnosisdev.com/rinkeby/orders/0x44a7611d0e7ca13bfc30d1211a8b91d6d7cc145006afd2deaed36ea6a635367eff714b8b0e2700303ec912bd40496c3997ceea2b61261a17

UPD: Nevertheless, the issue is related to tiny amount

fairlighteth · 2021-08-25T10:29:19Z

Styled the execution price, after merging in #1309 into orders-panel-10 -> then pull origin into this branch for testing:

mergify · 2021-08-25T10:29:24Z

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

mergify · 2021-08-25T10:29:25Z

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

fairlighteth · 2021-08-25T10:54:42Z

@elena-zh @alfetopito Does this have to do with the rounding issue from #1281 ?

mergify · 2021-08-25T10:55:14Z

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

fairlighteth · 2021-08-26T08:59:13Z

@elena-zh Can we merge this one? I think @alfetopito 's comment was addressed and the price issue is related to the other PR I believe.

elena-zh

Hi @biocom , yes, looks good to me.
I have created a separate issue for the 0 price edge case #1318

fairlighteth · 2021-08-26T09:53:21Z

@alfetopito Merging, as I believe your blocking comment was addressed. In case you find this was not the case, please raise this on #1198

alfetopito

Late approval 👍

* Orders panel component. * Props update. * Open ordersPanel on click connected wallet. (#1203) * WIP - Orders panel part 3 (Add wallet + orders to the sidebar) (#1210) * Open ordersPanel on click connected wallet. * Orders in side bar WIP. * Fix issue types * Revert "Fix issue types" This reverts commit cb95c89. * Fix issue with types 2 * Account details inside orders sidebar. * Fix import. * Fix folder name. Co-authored-by: Anxo Rodriguez <anxolin@gmail.com> * WIP - Orders panel part 4 (Re-style account details/activity/orders) (#1227) * Open ordersPanel on click connected wallet. * Orders in side bar WIP. * Fix issue types * Revert "Fix issue types" This reverts commit cb95c89. * Fix issue with types 2 * Account details inside orders sidebar. * Fix import. * Fix folder name. * Re-style of account details/activity. Co-authored-by: Anxo Rodriguez <anxolin@gmail.com> * WIP - Orders panel part 5 (More styling and tweaking) (#1234) * Open ordersPanel on click connected wallet. * Orders in side bar WIP. * Fix issue types * Revert "Fix issue types" This reverts commit cb95c89. * Fix issue with types 2 * Account details inside orders sidebar. * Fix import. * Fix folder name. * Re-style of account details/activity. * Orders sidebar styling. * Orders sidebar styling. * Orders sidebar styling. * Orders sidebar styling. * Orders sidebar styling. * Transaction update. * Transaction update. Co-authored-by: Anxo Rodriguez <anxolin@gmail.com> * WIP - Orders panel part 6 (Address feedback an wallet issues) (#1249) * Open ordersPanel on click connected wallet. * Orders in side bar WIP. * Fix issue types * Revert "Fix issue types" This reverts commit cb95c89. * Fix issue with types 2 * Account details inside orders sidebar. * Fix import. * Fix folder name. * Re-style of account details/activity. * Orders sidebar styling. * Orders sidebar styling. * Orders sidebar styling. * Orders sidebar styling. * Orders sidebar styling. * Transaction update. * Transaction update. * Fix wallet stuff. * Fix wallet stuff. * Mobile tx detail indent. * WIP - Orders panel part 7 (TBD) (#1250) * tablet size fix * Fix 1238 Co-authored-by: Anxo Rodriguez <anxolin@gmail.com> * Rename prop. * Orders Sidebar --> Slide in effect. (#1270) * Sidebar slide in. * Scroll fix. * Orders Sidebar --> Click to copy move. (#1269) * Click to copy move. * Icon wrapper remove. * Mobile header fixes. * Re-factor Copy component. * Cancelling label re-factor + Fix cancellation modal (#1293) * Cancelling label re-factor. * Fix close modal/sidebar click event. * Shimmer effect OPEN orders. * closeOrdersPanel for FAQ link. * Refert passing prop, instead open link new tab. * Update src/custom/components/AccountDetails/Transaction.tsx Co-authored-by: David <david.sato64@gmail.com> Co-authored-by: David <david.sato64@gmail.com> * Orders Sidebar --> Show limit prices + valid to/filled on dates (#1279) * Add limit price and valid to/filled date. * Add limit price and valid to/filled date. * Fixed type of getLimitPrice utils function * Execution price WIP. * Styled file for cleanup. * Styled file for cleanup. * Fix grid on mobile (Safari). * comment out getExecutedPrice * Expired order strike through. * Unfillable faq link external. * Price functions for Recent History (#1289) * Add bignumber, so library matches our dex-js * Add price utils * Refactor transaction summary * Change todo * Rename variable to executionPrice. * Delete comment * Remove comments and improve doc * Fix path and default value * Add datatype for api additional data (#1290) # Summary Continues #1289, Augment the order datatype so we can have the executed volumes. Adds the datatype where we can save the API information It also makes use of this data in the recent history. ## Not included Saving the actual data. Co-authored-by: biocom <michel@gnosis.pm> * Correction of buy/sell prices. * Close walletModal on connection success * Orders panel execution price (#1309) * Fix decimals bug * Moved additional info to BaseOrder, everything is serializable * Added additional info to order fullfilment type * Passing along additional info when fulfillig order * Storing additional info on order obj Co-authored-by: Leandro Boscariol <leandro.boscariol@gnosis.io> Co-authored-by: Leandro Boscariol <leandro.boscariol@gnosis.io> Co-authored-by: Anxo Rodriguez <anxolin@gmail.com> Co-authored-by: Leandro Boscariol <alfetopito@users.noreply.github.com> * Fix price format (#1310) * Add limit price and valid to/filled date. * Add limit price and valid to/filled date. * Fixed type of getLimitPrice utils function * Execution price WIP. * Styled file for cleanup. * Styled file for cleanup. * Fix grid on mobile (Safari). * comment out getExecutedPrice * Expired order strike through. * Unfillable faq link external. * Price functions for Recent History (#1289) * Add bignumber, so library matches our dex-js * Add price utils * Refactor transaction summary * Change todo * Rename variable to executionPrice. * Delete comment * Remove comments and improve doc * Fix path and default value * Add datatype for api additional data (#1290) # Summary Continues #1289, Augment the order datatype so we can have the executed volumes. Adds the datatype where we can save the API information It also makes use of this data in the recent history. ## Not included Saving the actual data. Co-authored-by: biocom <michel@gnosis.pm> * Correction of buy/sell prices. * Close walletModal on connection success * fix price format * Clean console.log * Clean console.log * Price invert fix. * Orders panel execution price (#1309) * Fix decimals bug * Moved additional info to BaseOrder, everything is serializable * Added additional info to order fullfilment type * Passing along additional info when fulfillig order * Storing additional info on order obj Co-authored-by: Leandro Boscariol <leandro.boscariol@gnosis.io> * Style execution price. * Fix transaction style for longer digits. Co-authored-by: Leandro Boscariol <leandro.boscariol@gnosis.io> Co-authored-by: Anxo Rodriguez <anxolin@gmail.com> Co-authored-by: Leandro Boscariol <alfetopito@users.noreply.github.com> * Price out of market iteration 2. (#1346) * Fix Safari sidebar style issue. Co-authored-by: Anxo Rodriguez <anxolin@gmail.com> Co-authored-by: David <david.sato64@gmail.com> Co-authored-by: Leandro Boscariol <leandro.boscariol@gnosis.io> Co-authored-by: Leandro Boscariol <alfetopito@users.noreply.github.com>

biocom and others added 16 commits August 18, 2021 15:35

Add limit price and valid to/filled date.

c10d80b

Add limit price and valid to/filled date.

c07c06b

Fixed type of getLimitPrice utils function

6294eb0

Execution price WIP.

ad86001

Styled file for cleanup.

e56f1cb

Styled file for cleanup.

daeba48

Fix grid on mobile (Safari).

55258ed

comment out getExecutedPrice

28e5ef0

Merge conflict.

d6b8afe

Expired order strike through.

6a7e6be

Unfillable faq link external.

9c83925

Pull/merge conflict.

ef56009

Correction of buy/sell prices.

9850f40

Close walletModal on connection success

141d546

fix price format

0533d76

fairlighteth requested review from alfetopito, anxolin, elena-zh, MareenG and W3stside August 24, 2021 17:01

biocom added 2 commits August 24, 2021 19:02

Clean console.log

cfe67c6

Clean console.log

2f6436a

anxolin approved these changes Aug 24, 2021

View reviewed changes

alfetopito suggested changes Aug 24, 2021

View reviewed changes

Price invert fix.

dbe3d60

biocom added 2 commits August 25, 2021 12:00

Merge branch 'orders-panel-10' of gno.github.com:gnosis/cowswap into …

6bf065b

…orders-panel-13

Style execution price.

0a2564b

This was referenced Aug 25, 2021

Orders Sidebar --> Show limit prices + valid to/filled on dates #1279

Merged

Connection modal is still displayed when change a connection type #1240

Closed

fairlighteth changed the base branch from orders-panel-10 to orders-panel-1 August 25, 2021 14:04

biocom added 2 commits August 25, 2021 16:10

Fix transaction style for longer digits.

7489836

Solve merge conflict.

bc156e3

elena-zh mentioned this pull request Aug 26, 2021

Swap/Side bar: Price is displayed as 0 when it is too tiny #1318

Closed

elena-zh approved these changes Aug 26, 2021

View reviewed changes

fairlighteth merged commit c558f35 into orders-panel-1 Aug 26, 2021

alfetopito reviewed Aug 26, 2021

View reviewed changes

alfetopito deleted the orders-panel-13 branch August 26, 2021 20:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix price format #1310

Fix price format #1310

fairlighteth commented Aug 24, 2021

github-actions bot commented Aug 24, 2021

anxolin Aug 24, 2021

fairlighteth Aug 24, 2021

fairlighteth Aug 24, 2021

alfetopito Aug 24, 2021

alfetopito commented Aug 24, 2021

alfetopito Aug 24, 2021

fairlighteth commented Aug 25, 2021

elena-zh commented Aug 25, 2021 •

edited

Loading

fairlighteth commented Aug 25, 2021 •

edited

Loading

mergify bot commented Aug 25, 2021

mergify bot commented Aug 25, 2021

fairlighteth commented Aug 25, 2021

mergify bot commented Aug 25, 2021

fairlighteth commented Aug 26, 2021

elena-zh left a comment

fairlighteth commented Aug 26, 2021

alfetopito left a comment

Fix price format #1310

Fix price format #1310

Conversation

fairlighteth commented Aug 24, 2021

Summary

To Test

github-actions bot commented Aug 24, 2021

anxolin Aug 24, 2021

Choose a reason for hiding this comment

fairlighteth Aug 24, 2021

Choose a reason for hiding this comment

fairlighteth Aug 24, 2021

Choose a reason for hiding this comment

alfetopito Aug 24, 2021

Choose a reason for hiding this comment

alfetopito commented Aug 24, 2021

alfetopito Aug 24, 2021

Choose a reason for hiding this comment

fairlighteth commented Aug 25, 2021

elena-zh commented Aug 25, 2021 • edited Loading

fairlighteth commented Aug 25, 2021 • edited Loading

mergify bot commented Aug 25, 2021

mergify bot commented Aug 25, 2021

fairlighteth commented Aug 25, 2021

mergify bot commented Aug 25, 2021

fairlighteth commented Aug 26, 2021

elena-zh left a comment

Choose a reason for hiding this comment

fairlighteth commented Aug 26, 2021

alfetopito left a comment

Choose a reason for hiding this comment

elena-zh commented Aug 25, 2021 •

edited

Loading

fairlighteth commented Aug 25, 2021 •

edited

Loading