Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix marker opacity of mostly null data series #1941

Merged
merged 10 commits into from
Dec 3, 2024
Merged

Fix marker opacity of mostly null data series #1941

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
49a3e4c
Fix marker opacity when most of the data is null
aestoltm Nov 4, 2024
7b352a4
Fix up spacing and remove debugging code
aestoltm Nov 4, 2024
47778e1
Revert to using previous logic but include check for null data point
aestoltm Nov 13, 2024
c267667
Make logic consistent for null trace check
aestoltm Nov 13, 2024
71d2f3e
Fix comparision for null point existence
aestoltm Nov 13, 2024
1a37b92
Merge branch 'xdmod11.0' into marker_bugfix
aestoltm Nov 22, 2024
469bf6f
Minor refactor on determining marker visibility for a plot
aestoltm Nov 22, 2024
c18daab
Adjust opacity logic because the value is a float and not boolean
aestoltm Nov 22, 2024
5386243
Merge branch 'xdmod11.0' into marker_bugfix
aestoltm Nov 27, 2024
feb4f6d
Merge branch 'xdmod11.0' into marker_bugfix
aestoltm Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion classes/DataWarehouse/Visualization/AggregateChart.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,7 @@ public function configure(
$drillable = array();
$text = array();
$colors = array();
$notNullCount = 0;

// to display as pie chart:
if($data_description->display_type == 'pie')
Expand Down Expand Up @@ -976,6 +977,9 @@ public function configure(
'values' => $yValues,
);

if (!is_null($value)) {
$notNullCount++;
}
}
// Dont add data labels for all pie slices. Plotly will render all labels otherwise,
// which causes the margin on pie charts with many slices to break
Expand Down Expand Up @@ -1016,6 +1020,10 @@ public function configure(
'id' => $yAxisDataObject->getXId($index),
'label' => $yAxisDataObject->getXValue($index)
);

if (!is_null($value)) {
$notNullCount++;
}
}
}

Expand Down Expand Up @@ -1087,7 +1095,8 @@ public function configure(
// Hide markers for 32 points or greater, except when there are multiple traces then hide markers starting at 21 points.
// Need check for chart types that this applies to otherwise bar, scatter, and pie charts will be hidden.
$hideMarker = in_array($data_description->display_type, array('line', 'spline', 'area', 'areaspline'))
&& ($values_count >= 32 || (count($yAxisObject->series) > 1 && $values_count >= 21));
&& ($values_count >= 32 || (count($yAxisObject->series) > 1 && $values_count >= 21))
&& $notNullCount > 1;

$trace = array_merge($trace, array(
'automargin'=> $data_description->display_type == 'pie' ? true : null,
Expand Down
3 changes: 2 additions & 1 deletion classes/DataWarehouse/Visualization/TimeseriesChart.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,8 @@ public function configure(
// Hide markers for 32 points or greater, except when there are multiple traces then hide markers starting at 21 points.
// Need check for chart types that this applies to otherwise bar, scatter, and pie charts will be hidden.
$hideMarker = in_array($data_description->display_type, array('line', 'spline', 'area', 'areaspline'))
&& ($values_count >= 32 || (count($yAxisDataObjectsArray) > 1 && $values_count >= 21));
&& ($values_count >= 32 || (count($yAxisDataObjectsArray) > 1 && $values_count >= 21))
&& $y_values_count > 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request comments talk about not null values, but this change does not appear to be related to not null. Please explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored the code we discussed. The logic for the marker visibility threshold is the same as during 10.5 which should have been the implementation from the start. The only difference is we did not track non-null (visible) data points for Aggregate charts in 10.5.


$isRemainder = $yAxisDataObject->getGroupId() === TimeseriesDataset::SUMMARY_GROUP_ID;

Expand Down