[bisq-network/bisq] Cleaner formatting for trading charts date axes (#4715)

Stan notifications at github.com
Fri Oct 30 15:55:48 UTC 2020


@ghubstan commented on this pull request.

I tested it, looks nice.  If I could (I'm not a JFX expert), I would center-align the labels where possible, e.g.,
```
29/Oct
 2020
```

But I would not block approval and merging without that.  
I will add a `Tested ACK` if the switch statement styling is changed.


> -                    return index % 4 == 0 ? DisplayUtils.formatDateAxis(new Date(time)) : "";
-                else
-                    return index % 3 == 0 ? DisplayUtils.formatTimeAxis(new Date(time)) : "";
+                String fmt = "";
+                switch (model.tickUnit) {
+                case YEAR  : fmt = "yyyy";
+                    break;
+                case MONTH : fmt = "MMMyy";
+                    break;
+                case WEEK  :
+                case DAY   : fmt = "dd/MMM\nyyyy";
+                    break;
+                case HOUR  :
+                case MINUTE_10: fmt = "HH:mm\ndd/MMM";
+                    break;
+                default:        // nothing here

You might want to reformat the style of the switch statement so `fmt = "..."` are on separate lines.  Not sure codacy will complain about this, but other devs might want the switch style to be consistent with the rest of the code base.
(This is the 1st time I've seen this style in Bisq;  I'm not saying there may be other examples in the code base.)

```
switch (model.tickUnit) {
                    case YEAR:
                        fmt = "yyyy";
                        break;
                    case MONTH:
                        fmt = "MMMyy";
                        break;
                    case WEEK:
                    case DAY:
                        fmt = "dd/MMM\nyyyy";
                        break;
                    case HOUR:
                    case MINUTE_10:
                        fmt = "HH:mm\ndd/MMM";
                        break;
                    default:        // nothing here
                }

```

-- 
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/4715#pullrequestreview-520786517
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20201030/85405494/attachment.html>


More information about the bisq-github mailing list