Fix for charts with side-by-side grouping (Bar/Area plots)#1883
Fix for charts with side-by-side grouping (Bar/Area plots)#1883jpwhite4 merged 36 commits intoubccr:xdmod11.0from
side-by-side grouping (Bar/Area plots)#1883Conversation
side-by-side grouping (Bar/Area plots)
…ame realm and multiple data sources from different realms in the same plot
…ad of using traces names (which are editable by users)
|
Comment to explain some of the proposed changes: Note: The term
|
| // for each of the dataseries on this yAxisObject: | ||
| foreach($yAxisObject->series as $data_description_index => $yAxisDataObjectAndDescription) | ||
| { | ||
| $legendRank += 2; |
There was a problem hiding this comment.
Note there is no trendline series for aggregate charts therefore this was supposed to increment by 1.
| && $usageGroupBy !== 'none' | ||
| ) { | ||
| $rank = $meDataSeries['legendrank']+1; | ||
| $rank = $meDataSeries['legendrank'] / 2; |
There was a problem hiding this comment.
We've had problems in the past using floats because their representation changes between php and js versions.
Is $rank supposed to be an int? is it guaranteed to be an int?
There was a problem hiding this comment.
I need to take another look at this. It doesn't seem to be guaranteed to be an int at the moment.
There was a problem hiding this comment.
$rank here is guaranteed to be an int after taking a closer look. This is only run when the series is a part of a timeseries plot and it is based off a variable we control, $legendRank.
During my investigation I noticed I was incrementing the $legendRank off by one. However, it was not noticeable because any series with a "tied" $legendRank would be broken by the fact that one of the traces came later in the data array passed with the plot. Therefore, I reverted the refactor to the Aggregate chart $legendRank I did last week and I put the timeseries $legendRank to increment by 3 (because we have the primary trace and two possible supplemental traces).
… notice on the plot because the next series (and its supplements plots) are placed later in the data array so I believe Plotly was placing them later in the legend for the same reason
| baseChartOptions.data.sort((trace1, trace2) => { | ||
| if (baseChartOptions.layout.barmode !== 'group') { | ||
| return Math.sign(trace2.zIndex - trace1.zIndex) || Math.sign(trace2.traceorder - trace1.traceorder); | ||
| return Math.sign(trace2.traceorder - trace1.traceorder); |
There was a problem hiding this comment.
Please change this if to explicitly check for a column bar chart (rather than not == group which will match scatter plots and pie charts too).
There was a problem hiding this comment.
This should be addressed.
For the trendline code that contained the ternaries, it seems that having the valid check was unnecessary. Nothing breaks when the log scale and std error bars are enabled with the trendline series. I need to start working on a few refactor PRs for Plotly because I saw a few cases where we can refactor barmode (only need for bar charts shouldn't be default on every chart) and allowing log scale with error bars (haven't found any reason why it shouldn't be allowed, I can look back on the git blame and try to find a reason)
Fix for charts with `side-by-side` grouping (Bar/Area plots)
Risk: MEDIUM
Notes on Risk: These changes affect Area/Bar plots due to both of these making use of the stacking feature. Minor changes to trace ordering which affects every plot. The change on trace ordering should be lower risk because the
zIndexis not being used due tolayeringbeing currently disabled.Severity: MEDIUM/HIGH
Notes on Severity: Depending on the units of the data and if the data sources are from the same realm or not the plot has misplaced traces, due to wrongful stacking (instead of side-by-side). See attached plots.
Description
Side by side bar charts were broken when comparing time-series data from two different realms.
Before Fix:
After Fix:
Found an additional bug with legend ordering w/ trendline and error bars enabled:
Before:
After:
Motivation and Context
Fixes time-series grouped (side by side) bar charts.
Tests performed
Tested on dev and tested grouped bar charts with data from the same realm to check for regressions
Checklist: