[24][TBAPI-0KA] Open Orders Tab: Group By "Trading Pair"

[fractalnode]

I would like to propose some improvements in Open Orders Tab I currently have many open orders on many diferent markets and I'm always looking to find answers for few questions:

• What is the sum of every base asset I want to sell or buy on each pair.
• What is the sum of every quote asset I want to sell or buy on each pair.
• What is the average order price on each pair.
• What is the current order value for each pair
• What is the expected value of the order for each pair.

Additionally I would like to be able to:

• select all orders and cancel them with one click (this works now)
• select all orders on the same pair and cancel them with one click

I am attaching two mockups below. I think that the accordion would be great for this purpose.

[froooze]

Good idea, can this be merged with #1666 ?

[startailcoon]

Very interesting idea, indeed. Would most likely be doable, and a good reason to look on a possible way of #1666.

Those mockups will be a good start for anyone looking at this.

Should include:

• [ ] Looking at possible ways to integrate a "Group By Asset" / "No Grouping" selector
• [ ] Group By Asset view

[TBAPI-0KA]

Please assign to me.

[startailcoon]

@TBAPI-0KA I've assigned you.

[gibbsfromncis]

@TBAPI-0KA, cc @startailcoon

I would like to ask you @TBAPI-0KA to follow my personal requirements when you work on a tasks which relates somehow to UI part:

1) We use only ant framework - http://ant.design but using our personal wrapper (style-guide): Live: https://styleguide.bitshares.org/#/dark Github: https://github.com/bitshares/bitshares-ui-style-guide - there you can find all integrated components which we support from ant.design framework.

If you can't find some component you need please let me know and I'll add them.

2) When you work with old components (like in your case) please replace all of them by ant components.

3) Please follow component-container pattern (for e.g. - https://medium.com/teamsubchannel/react-component-patterns-e7fb75be7bb0)

If it affects on your estimate please provide yours.

[gibbsfromncis]

@TBAPI-0KA Just for example in your case you should use 3 ant components:

• Collapse (to show/hide tables of pairs)
• Table (you should replace existed table by Table component from bitshares-ui-style-guide)
• Checkbox (to use Check All feature - https://ant.design/components/checkbox/#components-checkbox-demo-check-all)

[startailcoon]

As well described by Gibbs, this would indeed include changes of the existing components to bitshares-ui-style-guide (ANT). The hours on this is a rough estimate, so please return back if you find it not enough.

[TBAPI-0KA]

@startailcoon Sure. My typical workflow is to start the work, spent as little time as possible, and see if any estimate correction is required. From my conversation with @gibbsfromncis regarding this issue, I see that some refactoring of existing functionality (orders table should be transferred to ANT first) will be required additionally. At this point, I don't think that it will take more than 2 hours more, but let you know for sure when will dive deeper into the code.

[TBAPI-0KA]

@gibbsfromncis cc @startailcoon

I've spent around 3 hours until this point. Things look a little bit more complicated than I expected. There are some questions which are related to Ant usage and refactoring.

The first issue is related to the combination of Table and Collapse from Ant. On the one hand, Ant components are very smart and have a lot of properties. On the other hand, customization is very limited. For this task, the table header acts as a collapsible element. However, the Table component doesn't support such behavior. Putting Table inside of Collapsible component will break the relation between table header and body, in terms of DOM elements. I see the following ways how to handle this:

1. Put content inside Collapsible header separately, or as a separate table. This will break alignment between header and body, which, in my opinion, kinda breaks the original idea, where summary is displayed under each column. But that's the easiest way to go, and this approach will not increase estimate over 6 hours I mentioned initially.
2. Put a table with fixed columns width inside Collapsible as a header, and the same table with data as Collapsible content. This should help with alignment between header and body. However, it may break responsivity, or cause other UI issues. I'd not use this approach. Also, this requires a try to say will it affect estimate or not.
3. Not use Collapsible at all. Instead, wrap a Table inside our own reusable CollapsibleTable component. I'm not sure how extendable Ant components are. Perhaps, some dirty hacks will be required. Maybe, it's not even possible at all, without breaking primary Table functionality itself. Also, this will definitely increase the estimate a little. However, I'd give a try to this approach anyway, since it's strategically better - we can reuse this component in other places.

The second issue is more complicated. I've implemented some basic UI for the table, and thought that I all I need to do is to change data format, used in the current implementation, to the data format accepted by the Ant's Table. And I was quite surprised - instead of JS objects and function/services that do calculations on the original data, I found that data array is populated with a set of React elements, which accept 14 parameters, and, internally, is a large mix of calculations and layout. It's easier to show than explain: https://github.com/bitshares/bitshares-ui/blob/develop/app/components/Account/AccountOrders.jsx#L216-L234 https://github.com/bitshares/bitshares-ui/blob/develop/app/components/Exchange/MyOpenOrders.jsx#L102-L346 Again, I see the following options:

1. The Table contains the property named "expandedRowRender", which allows passing ReactNode as a table row. I can wrap existing OrderRow component. Not sure how smoothly this will work. Most likely, it will, but will require some styling fixes, and minor logic ones. In terms of implementation/estimate - this approach has the slightest impact. However, in fact, Ant components will be used only formally - OrderRow concentrates most of the logic here.
2. Do a basic refactoring of OrderRow, for our particular cases. I'd separate every single table cell into a separate component, and of course separate data calculation logic. It's more than 200 lines of code, so this will increase the estimate for at least 6 hours.
3. Do a complete refactoring. OrderRow is used in a few more places, i.e. My Orders on Exchange. I don't like passing variables like a bool one named "dashboard", which affects component rendering depending on the place used. However, this is much bigger and complicated task than the initial one and has almost no impact on the functionality. I can't even estimate it correctly at this point- it might be up to 20 hours and more.

Of course, it's possible to use the old approach (without Ant) for both issues. I don't know which are our priorities - do we need as many features as possible ASAP, or are we concentrated on refactoring and strategical benefits? @gibbsfromncis, could you look to this, please, and tell me what to do?

[gibbsfromncis]

@startailcoon this comment requires investigation from my part. I'll take a look on it more detailed today and provide you a summary

[gibbsfromncis]

@TBAPI-0KA Is it possible to use nested tables from ant - https://ant.design/components/table/#components-table-demo-nested-table as a solution for Table Collapse?

For TableRow let's use option #2 .

Please, before to start provide your estimate to finish this task

[startailcoon]

I see this implementation as a good thing, I support extended hours for this. Just post your rough estimate once you and Gibbs have a good idea on how to realize this issue.

Please use @gibbsfromncis to bounce ideas on this as he's well versed on Ant Components and can assist you.

[TBAPI-0KA]

@gibbsfromncis I've tried the Nested Tables approach. It looks really ugly even if one column of the nested table has different width:

However, I've built a PoC of custom CollapsibleTable component. In fact, it's almost completed, because I wanted to be sure that no hacks will be used. I was able to create such a reusable component, with nice code inside, and fully-supported API from regular Ant's Table.

So, regarding the estimate, here's a breakdown: Primary task functionality - 6 hours. This includes different data format calculation (partially), replacement of regular tables with the Ant ones, multiple tables with grouping, group order canceling per table, styling fixes. Collapsible Table component - 2 hours. Discussed earlier. Partial OrderRow refactoring - 6 hours. Includes splitting into different components between Account and other usages, data format calculation (second part), refactoring of separated Account component. Total - 14 hours. It's in terms of Qualified Hours - in fact, this will take around 16-20 hours in total.

[startailcoon]

If this comes out as good as you describe it, I'm all go for an increased estimate for it. Looking forward to see your work.

cc @gibbsfromncis @sschiessl-bcp

[TBAPI-0KA]

@startailcoon Well, it's always easy to check how good it comes out when the code is committed, right? Sorry for the delay - PR (#2686) is ready now. The short list of changes is presented in the description there.

However, in total it took me more time than I expected initially. 32 hours and 4 minutes of pure tracked development time, without counting communication, etc. I have a more detailed report with a breakdown, if necessary. There are a few reasons for such amount of time spent:

• some technical complications which come out on the way. I.e. one of the requirements from @gibbsfromncis was to use pure CSS animations for showing/hiding the table. However, it's not possible to animate the "display" property. Finding a more or less acceptable workaround took a decent amount of time. There were few similar issues - it could be seen in commits history.
• simply, the number of changes required. PR comparison shows 840 changed lines in just one file and almost 1500 in total. Of course, a significant part is copy-paste, but still, it's even hard to type such amount of text, even if you know exactly what to do.
• the last, but not least. I think @gibbsfromncis and @sschiessl-bcp are aware. Conflicting changes with table replacement come up from #2311, and those changes shouldn't be there. Fortunately, I had my implementation almost fully completed and was able to remove almost everything from conflicting changes. But still, this required manual conflict resolution on a few full files, to make sure nothing important is missing/broken, and this took time as well.

I'd like to stop on the latest one in more details, to explain why my implementation is fuller:

• The most important - data source contains data and only data. No layout pieces or React components. Layout, in its order, is placed in columns specification. This way, multiple grids with different data could be added in various places. In other words, reusability and design correctness is higher.
• Native Ant Table checkboxes and selection mechanism, including "mass select" feature, which was missing. For instance, selected items are maintained between different tables on the same page, which added zero cost to multiple tables canceling mechanism when the basic table was rewritten. Also, this reduced amount of custom code, putting this responsibility on a third-party component.
• Higher compatibility with Ant in general - i.e. no need to render tags manually, more control on layout/behavior through columns specification.
• Full compatibility with bitshares-styleguide, including fonts, checkboxes, etc.
• Preserved details from earlier implementation, such as cursors, footer formatting, elements alignment, click handlers, etc, but implemented in "Ant way".
• Cleaned up inside code from unused references/variables.
• Reusable functions for different tables rendering. These functions can be placed in separate files since they have a minimal amount of dependencies, and/or easily extended with various columns sets.

I understand that I extended the initial estimate quite a lot, and some of the core devs would do the same changes faster just because they know some common parts of the project much better than me. I feel uncomfortable to ask about something more, but, on the other hand, I also have a feeling that this is important and nicely done piece of work, which simplifies further support. So, I'm not asking about something significant. But if somebody of you guys can quickly look at the code, estimate the changes and how much you think they should fairly take, and there's a window for some extra hours - I'd really appreciate that.

Also, if possible, could this one be included in the 190510 milestone, please?

Some things (thoughts/known issues/questions) that lie out of the scope of this task, but are related:

• there's "account.group_by_asset" key added to the translation files. I've added English and Russian translations, but the rest should be changed appropriately.
• table footer is taken from a parent component. I've kept this approach for a not grouped table because I didn't found a quick way how to move render inside the reponsible component. Why it's important - because I have a feeling that this footer is kinda laggy. It shows only the total amount of all orders, and that number is fixed all the time. For instance, when orders count is higher than 20, paging enables, but footer still shows the total amount. For me, it's quite confusing. And for grouped tables, is just nonsense, because they show correct amount in the header. If this behavior should be changed, please create an issue or let me know, I'll create it by myself.
• click handlers on rows (changing the market direction) were kept intact. It works weird with grouped tables. By default, grouped tables are collapsed. If users uncollapse the table and click on any row, it will hide again. It's logical - because, in fact, it's a different table, with different market direction. But doesn't look nice from UX point of view.
• in general, this issue says nothing regarding show/collapse behavior. On the basic level, as it is now, with a large number of orders, page flickers all the time, when some orders are completed, then new are created, etc. It flickers in the non-grouping mode too, but it's much more obvious with grouped tables. It's possible, for instance, store every table collapse state separately, but it's a subject for another issue as well.
• column header width slightly changes when the table is collapsed. There's no way to align the column header with content when this content is hidden. Also, different tables have different content, which doesn't look very nice with a big amount of orders. I'm not sure how to better handle this. One of the possible solutions is to set fixed width in percents for every column. The difference with the approach which I stated as not very good earlier is that this time it's the same table, and header width is strictly bound to the width of the cell.
• looks like an obvious bug - after a page refresh, Open Orders tab stays opened, but no data is displayed, and layout is broken, until navigation to the other tab and back.

[sschiessl-bcp]

Thanks for the detailed description! Considering the change of scope, your learning curve and the refactoring issue, what is your suggestion as a bounty?

[TBAPI-0KA]

If you ask about facts, after all the fixes and changes, I've spent 34 hours and 32 minutes on development in total, and 4 hours and 45 minutes on the communication. A fair amount of hours lies out somewhere between 14 and 35. If you ask about my opinion, I think qualified dev with good project knowledge (I assume that's what you mean by learning curve here?) would save 2-4 hours because of this knowledge. Also, around 4-5 hours were spent on things that didn't work and weren't included in any commits - i.e. I've tried few approaches on Collapsible Table animation principle before finding acceptable one in terms of performance, best practices, code clarity, and complexity. The rest of the time was spent quite effectively.

[startailcoon]

It looks good, and it seems quick in response when used with an account with lots of open and active trades.

The new layout looks good imho. Only thing is, as you've noted, the headers of the collapsed tables that aren't aligned compared to each other. Due to the limitations and requirements of this issue I still find this acceptable. You've made a good result based on the limitations of the components, I'm impressed.

The non-grouped table looks good with the changes.

Since the work done was larger than expected, and I'm not surprised, I'd like to offer an increased bounty hour for you. Let me discuss with the team and Stefan, that did the reviewed, and I'll return with an update for you.

[TBAPI-0KA]

I agree with you regarding alignment. I think this should be fixed somehow. My idea was not to blow up the scope further but create smaller issues instead. This would give more clarity in terms of how much time which particular things take. If you like to, please create a separate issue, and you can assign it to me right over. I'll make all the changes required. However, the question is "how this should be changed?". Please refer to the second point of my comment on the PR discussion: https://github.com/bitshares/bitshares-ui/pull/2686#issuecomment-492079778 It contains much more details regarding why this happens, and what are the options. In short - empty space should be distributed by some rules. The only reason why you haven't noticed this earlier in the non-grouped mode is that some columns in this mode divide empty space between them. This way, these gaps aren't so noticeable, but they are still present.

[sschiessl-bcp]

We have discussed your issue and agreed to a bounty of 24. I hope this meets your expectations, please do voice if not.

We are very happy with your delivery and quality, and would be very happy if you continue to contribute!

[TBAPI-0KA]

I think it's a fair amount.

I'm glad you're happy with what I'm doing. I like to work on this project as well. For me, sometimes it's quite complicated to understand the exact requirements of the various issues, and I hardly understand how the priorities are defined. So, please ping me directly if you have any decent piece of work. I think with discussions with @gibbsfromncis I'm capable to do a task of almost any complexity. Except probably something which heavily relies on the internal business logic rules, but I'll tell you in this case before doing anything on it.

[sschiessl-bcp]

I think it's a fair amount.

I'm glad you're happy with what I'm doing. I like to work on this project as well. For me, sometimes it's quite complicated to understand the exact requirements of the various issues, and I hardly understand how the priorities are defined. So, please ping me directly if you have any decent piece of work. I think with discussions with @gibbsfromncis I'm capable to do a task of almost any complexity. Except probably something which heavily relies on the internal business logic rules, but I'll tell you in this case before doing anything on it.

This one comes to mind #1908