Add option to see orders of all coins in the order panel [WIP] #377
Conversation
|
I think we should add exchange and name column to the orders panel too? |
|
Yeah. Only when showing all orders. |
|
While digging the subscribe and update problem, I notice a bug in the coins panel. When you switch ticker, like currently in ltc and click btc, there will be three history change trigger (btc -> ltc -> btc)and result in three resubscribe. Are you aware of that? |
|
I wasn't, no. Can you post a screenshot from the browser web dev panel showing the messages? |
|
Here I switch from ltc to eos. |
|
We need to move dispatch out of history.listen to solve this. Maybe a middleware？ |
|
For the all orders update problem, maybe we subscribe order update for all subscribed coins like the ticker and move the order state out to a separate reducer like ticker too? |
|
Just to let you know: I will be busy for the next few days so may not have a chance to look at this until I have some home PC time. Looks interesting though - I think I may have seem symptoms of this issue in other places. |
|
Hey @lowwor - can we move the subscribe issue to a separate issue/PR and discuss there? Would make it easier to tackle one thing per PR. |
|
Yeah, see #394. |
|
Great, thanks. How's it going with this one? Can I help? |
|
Yeah, I am thinking of one situation, what if we delete the ticker in the ticker panel, then all orders may not include the deleted ones. |
| function orderDecorate(order, exchanges) { | ||
| return { | ||
| ...order, | ||
| exchangeMeta: exchanges.find(e => e.code === order.coin.exchange) |
There was a problem hiding this comment.
I'd do this find once and pass it to both decorate methods.
There was a problem hiding this comment.
Yeah, but for all coins' order, still need to find for each order, so I just left it there.
|
Not sure what you mean. Could you clarify? |
|
If we have orders of the deleted ticker, then we may not subscribe to the deleted ticker, so we would not get the orders of the deleted ticker. But this is a problem we already have even without this pr. Maybe we skip it here. |
badgerwithagun
changed the title
|
I will close this for now since we are in the refactor process. I will check back when we are stable again. |
|
@all-contributors add lowwor for code |
|
I've put up a pull request to add @lowwor! 🎉 |
|
@all-contributors add lowwor for userTesting |
|
I've updated the pull request to add @lowwor! 🎉 |
This pr resolve #373.
Add a tab in the orders panel to toggle options for showing orders for all coins.