[bisq-network/bisq] Fixed sorting in views with TableView instances with no implemented sort (#3319)

Christoph Atteneder notifications at github.com
Tue Nov 12 13:29:18 UTC 2019


ripcurlx requested changes on this pull request.

NACK - Did you test this changes once? Obviously not, as nothing worked as expected. Having PRs like this as a reviewer makes it quite frustrating as it consumes more time reviewing it than implementing it myself.

> +        volumeColumn.setComparator(Comparator.comparing(item -> item.offer.getPrice()));
+        volumeColumn.setSortType(TableColumn.SortType.ASCENDING);
+        tableView.getSortOrder().add(volumeColumn);
+

The list in the offer book chart view is just a teaser and the the volume column is set to sortable false which will make this code not working. So just revert the changes at this place.

> +        column.setComparator(Comparator.comparing(RolesListItem::getLockupDate).reversed());
+        column.setSortType(TableColumn.SortType.ASCENDING);
+        tableView.getSortOrder().add(column);

Why are you sorting this based on lockup date if the name is displayed without any date information displayed. This just makes it confusing for the user.

> +        column.setComparator(Comparator.comparing(MyReputationListItem::getAmount));
+        column.setSortType(TableColumn.SortType.ASCENDING);
+        tableView.getSortOrder().add(column);

Again pointless defining a string to be sorted for as it is the default behavior.

>          tableView.getColumns().add(column);
+        column.setComparator(Comparator.comparing(BondListItem::getAmount));
+        column.setSortType(TableColumn.SortType.ASCENDING);
+        tableView.getSortOrder().add(column);

Don't change the default sort order to amount. Atm it is sorted by lockup date and it should stay like that. And all this doesn't work at all as desired as you have applied sorting to a string value which is the default behavior.

> @@ -249,6 +251,9 @@ public void updateItem(final VoteListItem item, boolean empty) {
                         };
                     }
                 });
+        column.setComparator(Comparator.comparing(VoteListItem::getBlindVoteDate));
+        column.setSortType(TableColumn.SortType.DESCENDING);
+        votesTableView.getSortOrder().add(column);
         votesTableView.getColumns().add(column);

This has no effect as the columns are not sortable right now. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3319#pullrequestreview-315510976
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191112/15df6e4e/attachment.html>


More information about the bisq-github mailing list