Enhancement: Chart.getOptions function #15904
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Preparation for #15880
File size comparisonSizes for compiled+gzipped (bold) and compiled files.
|
Visual test results - All diffing samples already approvedSamples changed
|
Do you expect any issues with |
TorsteinHonsi
added
the
Changelog: Feature
pawelfus
approved these changes
May 16, 2023
|
Ready for merge, pending CI |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added new function, Chart.getOptions(), to get the current active configuration options for the chart.
Upgrade notes
For Highcharts Stock, the ordering of series and axes in the
chart.series,chart.xAxis,chart.yAxisandchart.axesarrays were changed. Previously the navigator series and axes were last of the initial items, and when adding items they were appended. From now on, the internal navigator series and axes are always last, so the index of items now corresponds to that of the current options.Some refactoring was made
chartuser options are now kept in sync with series and axis user options, so thatchart.xAxis[0].userOptions === chart.userOptions.xAxis[0]. If that is not the case, for example it is broken after an update, it should be considered a bug.chart.xAxisandchart.yAxisso that the indexing of the axes corresponds to that of the options.To do (critical, better to land fast)
chart.options.cleanRecursivewith the Responsive module'sgetCurrentfunction.getOptionsfunction (POC in studies)diffObjectsonchart.options(orchart.userOptions?) compared todefaultOptions.chart.options.yAxisandxAxis. Reconsider that. Tests need to be changed, but are there other consequences of removing them?getOptionsTo do (non critical, can wait)
getOptionsshoulddeepEqualthe initial options, possibly except some internal, unharmful modifications to the options. And except items that can be objects or arrays, likeseries,xAxisandyAxis. These should be in array form ingetOptions.colorAxison top level. Currently settingcolorAxis: {}enables the color axis. Test how themes withcolorAxisdefined affects charts. It may be that this needs to be changed altogether, for example by requiringcolorAxis: { enabled: true }.Highcharts.setOptionscurrently modifyingdefaultOptions, making it unusable for identifying the actual user options. We need a new property storing the default default options (factoryDefaults?).getOptionsin the exporting module? Low gain, high risk. Probably now worth it.userOptions. The original intention of this property was to distinguish between actual user options and default options. But there has been an inflation in its usage. In most cases it could simply be replaced byoptions, that should always be up to date. In other cases, we can usecleanRecursiveto find out whether an option is set by the user or not. For complex objects, like axes and series, theoptionsare built in multi-level merges. In those cases, we probably need to split the merge so that it is possible to built a composite default options object to be able to diff out the user options.userOptionsinside thetooltip.options. Should be refactored.