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.

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

[ML] Fixes clean up of multi-bucket markers on Single Metric chart up… #24080

Merged

Conversation

peteharverson
Copy link
Contributor

@peteharverson peteharverson commented Oct 16, 2018

Summary

Fixes issue introduced in #23746 which could cause misplaced multi-bucket anomaly markers to appear in the Single Metric Viewer chart:

multibucket_marker_extra

Previously when the chart was redrawn, for example by changing the position or extending the range of the selection 'slider', the d3 selection used to clean out previously rendered chart markers was not specific enough for the 'single bucket' or 'multi-bucket' markers, so old markers could end up being re-rendered on the chart. This has been fixed by using a specific multi-bucket class for the d3 selection of the multi-bucket markers.

The fix was extended to the Anomaly Explorer single metric chart for consistency although these charts were not affected by the same issue as they are completely redrawn on container resize.

The size of the 'cross' symbol used for the multi-bucket markers has also been dropped slightly to reduce the visual impact of these markers, giving them dimensions similar in size to the 'single bucket' markers.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@peteharverson peteharverson added bug Fixes for quality problems that affect the customer experience review non-issue Indicates to automation that a pull request should not appear in the release notes v7.0.0 :ml Feature:ml-results legacy - do not use v6.5.0 labels Oct 16, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM
There's absolutely no chance of these shapes forming accidental rocket ships

@elasticmachine
Copy link
Contributor

💔 Build Failed

@peteharverson
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@peteharverson peteharverson force-pushed the ml-multi-bucket-marker-select branch from 85c547d to 25ef23b Compare October 17, 2018 08:31
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM ⚡️

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Anomaly Detection ML anomaly detection Feature:ml-results legacy - do not use :ml non-issue Indicates to automation that a pull request should not appear in the release notes review v6.5.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants