Skip to content

[XY axis] Fixes bug on bar chart appearing as stacked when the normal mode is selected#90602

Merged
nickofthyme merged 2 commits intoelastic:masterfrom
stratoula:fix-normal-mode-xy-axis
Feb 11, 2021
Merged

[XY axis] Fixes bug on bar chart appearing as stacked when the normal mode is selected#90602
nickofthyme merged 2 commits intoelastic:masterfrom
stratoula:fix-normal-mode-xy-axis

Conversation

@stratoula
Copy link
Copy Markdown
Contributor

@stratoula stratoula commented Feb 8, 2021 •
edited by nickofthyme
Loading

Summary

Closes #90599.
When I create a new bar chart with the new XY axis plugin, changing the mode to normal doesn't affect it. The bar chart continues to be displayed as stacked. This PR fixes it.

Screen Recording 2021-02-09 at 12 07 38 PM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@stratoula stratoula changed the title [XY axis] Fixes normal mode setting [XY axis] Fixes bug on bar chart appearing as stacked when the normal mode is selected Feb 8, 2021
@stratoula
Copy link
Copy Markdown
Contributor Author

stratoula commented Feb 8, 2021

@nickofthyme wdyt? I think that the enableHistogramMode should also take into consideration the mode setting.

@stratoula stratoula requested a review from nickofthyme February 8, 2021 15:29
@nickofthyme
Copy link
Copy Markdown
Contributor

nickofthyme commented Feb 8, 2021 •
edited
Loading

Ugh, I don't think it's that simple. From your image, it looks like the enzone is now computed wrong. Let me take a look later today and see.

@stratoula
Copy link
Copy Markdown
Contributor Author

stratoula commented Feb 8, 2021

Perfect! Thank you Nick ❤️

@nickofthyme
Copy link
Copy Markdown
Contributor

nickofthyme commented Feb 9, 2021 •
edited
Loading

Ok this a little tricky. The issue is using histogram mode forces stacked bars, so there is a check to determine when we can use the histogram mode without forcing it, namely when every bar series is stacked, or where there is only a single bar series. But the second condition just ignored the stacked check, not sure what I was thinking at the time, but that is wrong, it should check all bar series no matter how many there are.

So removing that second condition shows the normal bars as expected. Also checked with multiple bars series.

Screen Recording 2021-02-09 at 12 07 38 PM

The fix you had would have worked but the enableHistogramMode is used in other places like the endzone to determine the correct annotations. Sorry for force pushing 🙃 .

@nickofthyme nickofthyme force-pushed the fix-normal-mode-xy-axis branch from 8783040 to 5ab49e2 Compare February 9, 2021 18:18
@nickofthyme nickofthyme marked this pull request as ready for review February 9, 2021 18:19
@nickofthyme nickofthyme requested a review from a team February 9, 2021 18:19
nickofthyme
nickofthyme approved these changes Feb 9, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Let me know if you want me to put up a pr so that you can approve and merge.

@stratoula stratoula added Feature:XYAxis XY-Axis charts (bar, area, line) bug Fixes for quality problems that affect the customer experience v7.12.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Feb 11, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Feb 11, 2021

Pinging @elastic/kibana-app (Team:KibanaApp)

@stratoula
Copy link
Copy Markdown
Contributor Author

stratoula commented Feb 11, 2021

@elasticmachine merge upstream

stratoula
stratoula commented Feb 11, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

It works great Nick, I tested it and it solves the problem. I have tested various scenarios and it works. Thank you a lot, I assigned the bug to you as you solved it 🙂 I can't approve as it was originally my PR but feel free to merge it!

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 11, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
visTypeXy 140.8KB 140.7KB -48.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@nickofthyme nickofthyme merged commit 847d57b into elastic:master Feb 11, 2021
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 15, 2021
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 15, 2021

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 90602 or prevent reminders by adding the backport:skip label.

stratoula added a commit to stratoula/kibana that referenced this pull request Feb 15, 2021
@stratoula
[XY axis] Fixes bug on bar charts preventing unstacked mode (elastic#…
624621e 
…90602)
@stratoula stratoula mentioned this pull request Feb 15, 2021
Merged
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 16, 2021
stratoula added a commit that referenced this pull request Feb 16, 2021
@stratoula
[XY axis] Fixes bug on bar charts preventing unstacked mode (#90602) (#…
2050823 
…91414)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickofthyme nickofthyme nickofthyme approved these changes

Assignees

No one assigned

Labels

bug Fixes for quality problems that affect the customer experience Feature:XYAxis XY-Axis charts (bar, area, line) release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.12.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[XY Axis] Bar chart appears as stacked even though the normal mode is selected

4 participants

@stratoula @nickofthyme @elasticmachine @kibanamachine