[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