Skip to content

Use xdmod10.5 logic for enabling markers#1997

Merged
aestoltm merged 4 commits intoubccr:xdmod11.0from
aestoltm:bugfix_show_marker
Mar 6, 2025
Merged

Use xdmod10.5 logic for enabling markers#1997
aestoltm merged 4 commits intoubccr:xdmod11.0from
aestoltm:bugfix_show_marker

Conversation

@aestoltm
Copy link
Copy Markdown
Contributor

@aestoltm aestoltm commented Mar 6, 2025

Description

@aaronweeden found examples where series incorrectly had markers rendered. I looked at the changes make in #1941 and noticed it did not use the same marker logic as in xdmod10.5 (highcharts implementation) which matches our desired behavior. This PR corrects the logic that enables markers to be the same as in 10.5 timeseries and aggregate charts

Motivation and Context

Improve the clarity of charts. The incorrectly rendered markers made it difficult to see other series on the same date.

Tests performed

Tested on my dev port.

Checklist:

  • The pull request description is suitable for a Changelog entry
  • The milestone is set correctly on the pull request
  • The appropriate labels have been added to the pull request

@aestoltm aestoltm added bug Bugfixes Category:Metric Explorer Metric Explorer / Usage labels Mar 6, 2025
@aestoltm aestoltm added this to the 11.0.1 milestone Mar 6, 2025
@aestoltm aestoltm requested a review from aaronweeden March 6, 2025 16:47
@aestoltm aestoltm changed the title Use xdmod10.5 implementation for enabling markers Use xdmod10.5 logic for enabling markers Mar 6, 2025
aaronweeden
aaronweeden previously approved these changes Mar 6, 2025
break;
}
}
if($allNull)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if($allNull)
if($visiblePoints > 0)

and can get rid of $allNull variable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it should be if($visiblePoints == 0) so that we know its "allNull" and can skip.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're right!

@aestoltm aestoltm merged commit 0f6748a into ubccr:xdmod11.0 Mar 6, 2025
4 checks passed
@aestoltm aestoltm deleted the bugfix_show_marker branch March 6, 2025 23:27
aaronweeden pushed a commit to aaronweeden/xdmod that referenced this pull request Mar 7, 2025
* Use xdmod10.5 implementation of deciding when to show markers. I thought I did this in ubccr#1941 but it is not the same logic.
aaronweeden pushed a commit to aaronweeden/xdmod that referenced this pull request Mar 7, 2025
* Use xdmod10.5 implementation of deciding when to show markers. I thought I did this in ubccr#1941 but it is not the same logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bugfixes Category:Metric Explorer Metric Explorer / Usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants