Skip to content

Only show open orders on exchange view#484

Merged
sindresorhus merged 1 commit into
masterfrom
open-orders
Aug 27, 2018
Merged

Only show open orders on exchange view#484
sindresorhus merged 1 commit into
masterfrom
open-orders

Conversation

@lukechilds
Copy link
Copy Markdown
Member

Extends #481. Just targeting the #481 branch for now so you can easily see what's changed in the diff/commits. If we decide to merge this it should be merged into master after #481.

This PR changes the "Swaps" section of the Exchange view to "Open Orders". This is much more inline with how centralised excahnges work and makes more sense with GTC style orders. It's actually quite similar to the GDAX/Coinbase Pro UI so hopefully will seem more familiar to users.

Only open orders are listed in the exchange view. You can still filter the open orders down to the current trading pair. If you cancel them they will be removed from that view. One the order has been filled it will be removed from that view.

You can then view cancelled/completed orders in the Trades view.

Here an example of how it works:

open orders demo

I make a reasonable CHIPS/KMD order and a super low MYTH/KMD order that will never match. These are the only orders listed, none of my previous orders are shown here, they are in the Trades view. I can filter down to trading pair to only view open orders in the current trading pair. The CHIPS order matches and starts swapping, the MYTH order remains unmatched. It will keep rebroadcasting every 10 minutes until it gets matched or until I cancel it. I then cancel it and it is removed from open orders. Once the CHIPS trade completes it will be removed from the "Open Orders" view (not shown in GIF)

I was really keen on implementing this but after testing it for a while I'm not 100% sure we should definitely merge it, there are a few caveats:

Cancel behaviour is buggy (#262)

The cancel buttom seems to work inconsistently, I think this is due to the mm code only referencing one pending trade internall at a time in a global variable. Sometimes it seems to work ok but other times it doesn't.

Swap modal will disappear once order is complete

If you have the swap modal open while the order is going through, it will dissapear as soon as it's complete. This is because the swap list component is no longer rendered becasue it's no longer pending and the model component is a child compenent of that so it too stops rendering.

It's a bit confusing that you would be on swap 4/5 and then modal suddenly dissapears. We could probably fix this be component heirachy around a bit so the modal can still be visible when the order list item dissapears.

Trades will sometimes randomly dissapear and come back due to mm bug (jl777/SuperNET#956)

There is a bug in mm where we randomly get a failed/cancel event for a trade that hasn't failed/cancelled. Obvisouly we don't know the message is incorrect so we think the trade has failed and remove it from "Open Orders". Then, soon after we get another update from mm saying the swap progress has progressed and it will re-appear in "Open Orders" again.

This is pretty confusing because the order just dissapears and then re-appears. Also if the user has the swap modal open it will jsut close and the swap will be missing, only for it to re-appear after.

This will hopefully be fixed in mm soon (@jl777 is too busy on another project but I think @artemii235 may be taking a look into it). Even so, there is a hacky workaround we can implement that just ignore cancel events for trades that are in progress, as these should never be able to be cancelled.

I will implement a seperate PR for that and we can decide if we want to implement it.

@lukechilds lukechilds requested a review from a team August 22, 2018 17:18
@sindresorhus
Copy link
Copy Markdown
Contributor

Swap modal will disappear once order is complete

A simple fix for this could be to just not remove the swap if it's the currently opened one.

@sindresorhus sindresorhus changed the base branch from gtc to master August 27, 2018 09:45
@sindresorhus sindresorhus changed the title Only Show open orders on exchange view Only show open orders on exchange view Aug 27, 2018
@sindresorhus
Copy link
Copy Markdown
Contributor

With this, I think we should no longer limit the number of swaps in the list as performance is no longer a concern (because of react-virtualized) and because it will only be open orders.

@sindresorhus
Copy link
Copy Markdown
Contributor

Cancel behaviour is buggy (#262)

This is fixed.

Swap modal will disappear once order is complete

I will fix this.

Trades will sometimes randomly dissapear and come back due to mm bug

We can use your workaround in the mean time.

@sindresorhus sindresorhus merged commit a95f256 into master Aug 27, 2018
@sindresorhus
Copy link
Copy Markdown
Contributor

Merging as this is a great change and I need it to unblock some other changes.

@sindresorhus sindresorhus deleted the open-orders branch August 27, 2018 11:04
sindresorhus added a commit that referenced this pull request Aug 31, 2018
And instead of accepting a swap object, it now accepts a swap ID.

The swap modal is now controlled by the swap list instead of controlling itself.

This fixes the issue with an open swap modal suddenly disappearing when the swap completes, as mention in #484.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants