[bisq-network/bisq] Feature to enable disputes user avatars and tag editing (#5559)

Kai notifications at github.com
Wed Jun 9 09:02:17 CEST 2021


@KaiWitt commented on this pull request.



> +        String accountAgeTooltip = isFiatCurrency ?
+                accountAge > -1 ? Res.get("peerInfoIcon.tooltip.age", DisplayUtils.formatAccountAge(accountAge)) :
+                        Res.get("peerInfoIcon.tooltip.unknownAge") :
+                "";

A dedicated method for this inner ternary would be nice. Makes it readable and the method could also be used in PeerInfoIconDispute.java line 40



> +        Color ringColor = Color.rgb(0, 225, 0);
+        if (isFiatCurrency) {
+            switch (accountAgeWitnessService.getPeersAccountAgeCategory(hasChargebackRisk(trade, offer) ? signAge : accountAge)) {
+                case TWO_MONTHS_OR_MORE:
+                    ringColor = Color.rgb(0, 225, 0); // > 2 months green
+                    break;
+                case ONE_TO_TWO_MONTHS:
+                    ringColor = Color.rgb(0, 139, 205); // 1-2 months blue
+                    break;
+                case LESS_ONE_MONTH:
+                    ringColor = Color.rgb(255, 140, 0); //< 1 month orange
+                    break;
+                case UNVERIFIED:
+                default:
+                    ringColor = Color.rgb(255, 0, 0); // not signed, red
+                    break;
+            }
+        }
+        return ringColor;
+    }

I would prefer to immediately return `ringColor` if `!isFiatCurrency`, then you wouldn't have to nest the switch in the if clause. Lower level of nestedness is more pleasant to read. Might be a bit too opinionated, feel free to ignore/resolve

> +    }
+
+    protected Color getRingColor(Offer offer, Trade trade, Long accountAge, Long signAge) {
+        // outer circle
+        // for altcoins we always display green
+        Color ringColor = Color.rgb(0, 225, 0);
+        if (isFiatCurrency) {
+            switch (accountAgeWitnessService.getPeersAccountAgeCategory(hasChargebackRisk(trade, offer) ? signAge : accountAge)) {
+                case TWO_MONTHS_OR_MORE:
+                    ringColor = Color.rgb(0, 225, 0); // > 2 months green
+                    break;
+                case ONE_TO_TWO_MONTHS:
+                    ringColor = Color.rgb(0, 139, 205); // 1-2 months blue
+                    break;
+                case LESS_ONE_MONTH:
+                    ringColor = Color.rgb(255, 140, 0); //< 1 month orange

```suggestion
                    ringColor = Color.rgb(255, 140, 0); // < 1 month orange
```

> +                accountAge > -1 ? Res.get("peerInfoIcon.tooltip.age", DisplayUtils.formatAccountAge(accountAge)) :
+                        Res.get("peerInfoIcon.tooltip.unknownAge") :
+                "";
+        tooltipText = hasTraded ?
+                Res.get("peerInfoIcon.tooltip.trade.traded", role, fullAddress, numTrades, accountAgeTooltip) :
+                Res.get("peerInfoIcon.tooltip.trade.notTraded", role, fullAddress, accountAgeTooltip);
+
+        createAvatar(getRingColor(offer, trade, accountAge, signAge));
+        addMouseListener(numTrades, privateNotificationManager, trade, offer, preferences, useDevPrivilegeKeys,
+                isFiatCurrency, accountAge, signAge, peersAccount.third, peersAccount.fourth, peersAccount.fifth);
+    }
+
+    protected Color getRingColor(Offer offer, Trade trade, Long accountAge, Long signAge) {
+        // outer circle
+        // for altcoins we always display green
+        Color ringColor = Color.rgb(0, 225, 0);

Would be nice to have all used colors somewhere as variables or as a Enum so that we wouldnt have to comment the color value all the time

-- 
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/5559#pullrequestreview-679263003
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20210609/7d911dd4/attachment-0001.htm>


More information about the bisq-github mailing list