[bisq-network/bisq] Add missing filtering on lists (PR #5963)

[chimp1984]

@chimp1984 requested changes on this pull request.



> @@ -392,6 +407,19 @@ protected void deactivate() {
     // UI handlers
     ///////////////////////////////////////////////////////////////////////////////////////////
 
+    private void applyFilteredListPredicate(String filterString) {
+        filteredList.setPredicate(item -> {
+            if (filterString.isEmpty())
+                return true;
+
+            if (item.getBalance().toString().contains(filterString)) {

This does not cover formatted string. E.g. if balance is 0.01 BTC and you type in 0.01 it is not found.
Better create a new string field in the itme (formatter.formatCoin(this.balance) which reflects the displayed string

>  <VBox fx:id="root" fx:controller="bisq.desktop.main.funds.deposit.DepositView"
       spacing="10" xmlns:fx="http://javafx.com/fxml">
     <padding>
         <Insets bottom="15.0" left="15.0" right="15.0" top="15.0"/>
     </padding>
-
+    <HBox spacing="5.0">

The usage of FXML is a old left-over from early dev state. We should avoid adding new FXML items and add it in the normal View classes instead. This makes code easier to reuse (e.g. you can make a filter component which is then used every time when you show that filter). Beside better code reusablility and flexibility with more dynamic use cases FXML is slow when getting rendered, thats why we move away from it. We just never spent the time to cleanup the left overs and it would require some more work to remove it completely, so we use it as container still but do all the rest in the view class.

> +                }
+                if (trade.getDepositTxId() != null && trade.getDepositTxId().contains(filterString)) {
+                    return true;
+                }
+                if (trade.getPayoutTxId() != null && trade.getPayoutTxId().contains(filterString)) {
+                    return true;
+                }
+
+                Contract contract = trade.getContract();
+
+                boolean isBuyerOnion = false;
+                boolean isSellerOnion = false;
+                boolean matchesBuyersPaymentAccountData = false;
+                boolean matchesSellersPaymentAccountData = false;
+                if (contract != null) {
+                    isBuyerOnion = contract.getBuyerNodeAddress().getFullAddress().contains(filterString);

Those predicates appear several times. Would be good to make it reusable code

> +                    Set<Tradable> tradables = tradableRepository.getAll();
+
+                    TransactionAwareTradable maybeTradable = tradables.stream()
+                            .map(tradable -> {
+                                if (tradable instanceof OpenOffer) {
+                                    return new TransactionAwareOpenOffer((OpenOffer) tradable);
+                                } else if (tradable instanceof TradeModel) {
+                                    return new TransactionAwareTrade(
+                                            (TradeModel) tradable,
+                                            arbitrationManager,
+                                            refundManager,
+                                            btcWalletService,
+                                            pubKeyRing
+                                    );
+                                } else {
+                                    return new DummyTransactionAwareTradable(tradable);

I know that is from old code, but as that case never gets executed I think we should return null here and then filter out null in the next step. Tradable is only extended from OpenOffer and TradeModel (one other case is not relevant here). The DummyTransactionAwareTradable can be removed then.


-- 
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/5963#pullrequestreview-858553074
You are receiving this because you are subscribed to this thread.

Message ID: <bisq-network/bisq/pull/5963/review/858553074 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20220120/baa65f86/attachment.htm>