core
core copied to clipboard
Better nvd3 graph extents in Reporting
- [x] I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
- [x] I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue
Is your feature request related to a problem? Please describe.
This issue has two intertwined parts:
- Graphed values can exhibit dramatic visual variance when in fact they are quite close, so do not warrant the dramatic appearance (at least not by default). They can also be misleading in that some values appear low because they are close to the lower extent of the graph, but are high in absolute terms.
- Graph extents are cropped at whatever the min & max. values are, which is less clear than if they are rounded to sensible multiples so that top and bottom lines are shown, and the divisions are uniform.
Describe the solution you'd like
-
Add an option toggle (just like the Inverse one) to the graph UI that forces the Y-axis to anchor the scale to
0
. Ideally it would be on by default. The code for this is small:chart.forceY([Math.min(0, min), Math.max(0, max)]);
where
min
andmax
are already known or obtained fromd3.extent()
. I think it can be reverted with this:chart.forceY([]);
-
Round the extents to a sensible multiple so that the horizontal lines are evenly spaced. With a function like this:
function sensibleExtents(min, max) { const largestMagnitude = Math.max(Math.abs(min), Math.abs(max)); // largest power of 10 below the largest magnitude let division = Math.pow(10, Math.ceil(Math.log10(largestMagnitude)) - 1); // extra clamping to prevent wasted space when magnitude is barely larger than division, e.g. an extent of 200 on a max. of 105 if (largestMagnitude < (1.5 * division)) { division /= 10; } return [ division * Math.floor(min / division), division * Math.ceil(max / division) ]; }
we get
sensibleExtents(-12345, 432)
->[-13000, 1000]
, andsensibleExtents(0.05, 0.18)
->[0, 0.2]
.Combining that with (1):
chart.forceY(sensibleExtents(Math.min(0, min), Math.max(0, max)));
and
chart.forceY(sensibleExtents(min, max));
Describe alternatives you considered
A personal JavaScript bookmarklet that adjusts the graphs for me, but would not benefit others.
Additional context
Possibly related to #6527, but that is about configurable extents, this is about sensible, dynamic defaults.
Compare these examples of a set with automatic extents vs forced:
@Roy-Orbison if you feel up to to the task, would you mind opening a pull request?
Possibly. I was hoping to avoid setting up a build environment/VM for a small JS change. Maybe I could have a go and someone who knows that code could polish it up?
if you start with a PR, I'll review and see if we can polish it a bit. sure