Skip to content

Commit

Permalink
Root Cause Fix: Prevent label generation from stopping one label too …
Browse files Browse the repository at this point in the history
…early due to cumulative precision errors in JS float representations. This is also the root cause of issue apexcharts#430, allowing the removal of the previous fix.

Root Cause Fix: getMinYMaxY():
1) not determining chart min and max according to series type, thus
returning incorrect values.
For example, e2e test "candlestick/candlestick-line.html" has
chart.type "line" while the two series' have types "line" and
"candlestick" respectively. The candlestick data min/max was being
determined as for a "line" series.
2) Erroneous if conditional logic allowing entry to all chart types
rather than just the targeted types.

Exclude line and area charts from +/-5% range expansion that is applied
to accommodate box and candlestick elements. See below: root cause fix.

Remove the unnecessary range expansion. Root cause fixed.

Several locations: remove code from previous fixes for the following
issues, as the examples provided in those issue reports display
correctly now:

Previous fix for issue apexcharts#492 is obsolete, possibly following
commit #4562468 that fixed cases where the Y axis missed including the
uppermost tick due to JS precision errors.

If the user defines options that are consistent amongst themselves,
apply them without adjustment.

In computing the provisional nice step size, set magMsd only to integer
subdivisions of 10, i.e. magPow*[1,2,5,10], eliminating unhelpful
values like magPow*[3,4,6,7,8,9]. This can be overridden by setting
stepSize and setting forceNiceScale: true. Then, if the given
stepSize is bigger than the range, or impractically small and
resulting in excessive ticks, apply it after normalizing it the chart
order of magnitude and checking again for consistency.
E.g. if the range = 0.4 and user sets stepSize: 8 (or 8000 or 0.0008),
then stepSize is obviously disproportionate and impractical.
Normalize it to 0.08 or 0.008 depending on tickAmount etc.

Rename the class in Scales.js from "Range" to "Scales" to avoid
being confused with the "Range" class defined in Range.js.

Modified unit tests:
Brought into line with changes introduced in the current branch
to prevent trivial failure reports.
New unit tests to verify priority of user defined options.

New e2e sample to verify duplicate label removal by formatter. AFAICS,
the only way to hide labels that don't correspond to tick values is
via the formatters. This is the cause of duplicate values on axes.

All working except for brush-charts, which don't call niceScale.

Add utility functions:
	getGCD() - return GCD of two numbers (not used in this version)
	getPrimeFactors() - return all prime factors of an integer
	mod() - a mod b: to overcome precision errors

Reduce ticks nicely as chart svg size gets smaller, if options allow.

Multi series/multi yaxis charts: attempt force consistent
tickAmounts so they line up with the grid. Per series user tickAmount
override this.

Adjust some tickAmount values in e2e samples now that user defined
tickAmount is honoured unless it conflicts with other user defined
options.

Set default ticks back to 10. With the proposed changes, the default
value only influences the computed nice step size and is reduced to
best fit the data after that.

The previous default of 5 has the following visual effect:
Most charts work with base 10 numbers, and 10/5=2, which tends to
produce tick spacings of 0.2, 2, 20, etc. This tends to visually
squeeze graphs toward the centre of the chart, leaving more of the
peripheral chart area unused, lowering resolution. Having 10 as the
default reverses this tendency, bringing data points closer to the
extremities of the range.

defaults.js: remove the default tickAmount = 6 for polarArea charts.
Let niceScale determine it if not user defined.

radar-with-polygon-fill.xml: remove user defined tickAmount = 7. Let
niceScale() determine it, otherwise 7 ticks are what will be drawn,
which will produce a sub-optimal chart for the sample data.

multiple-yaxes.xml: associate y axes with their correct series'.
The visual change in the chart is that the Income yaxis is scaled
correctly. Further highlights the other changes in this commit, showing
the alignment of all y scale labels with the grid.

Add NetBeans IDE config and ignore private resources.

Core.setupBrushHandler():
Bug fix: needed to clone targetChart.w.config.yaxis, not w.config.yaxis.
Removed call to Scales.autoScaleY() and the assignment of results to
the target chart yaxis.
We no longer clobber the yaxis min and max elements, leaving them
for the user if they want to define them.
Only needs to record the event xaxis min and max for the target chart.

Scales.autoScaleY():
Removed. Not required.
Range.setYRange() and Range.getMinYMaxY() are now aware of xaxis
min max range from the brush selection event. These functions are
called as part of the target chart redrawing following the
triggering of the selection event loop in Core.setupBrushHandler().

Range.getMinYMaxY():
Work with brush X ranges.

Range.setYRange():
Allow mulitple Y axes to be scaled with Y ranges corresponding to
their independent data series'.

Brush now works with all appropriate chart arrangements, including
multiple series, multiple axes, collapsible series', combos, etc.

line/brush-charts.xml:
Modified to demonstrate brush charts with multiple collapsible series
and multiple Y axes, with dynamic "nice" scaling of axes.

Unit test: yaxis-min-max-spec.js
Specific Test: "min and max functions in multiple yaxis should return
correct values in params".
Charts with multiple Y axes can have different min values. Other
changes in this patchset ensure "nice" scaling plus alignment of ticks
with grid lines. Users can set minimums and maximums as per axis
options to override the default auto-ranging.
  • Loading branch information
rosco54 committed Feb 11, 2024
1 parent 5cd2811 commit bf0e5d1
Show file tree
Hide file tree
Showing 202 changed files with 5,018 additions and 6,502 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ tests/e2e/diffs
.nyc_output
.vscode
bundle-analysis.html
/nbproject/private/
6 changes: 3 additions & 3 deletions dist/apexcharts.common.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dist/apexcharts.esm.js

Large diffs are not rendered by default.

Loading

0 comments on commit bf0e5d1

Please sign in to comment.