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

[Lens] fix dual axis bar overlap #73680

Merged
merged 1 commit into from
Jul 30, 2020

Conversation

nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Jul 29, 2020

Summary

Fixes #72889

Upgrades @elastic/charts to 19.8.2 which includes only this fix (elastic/elastic-charts@v19.8.1...v19.8.2)

Before

image

After

Screen Recording 2020-07-29 at 09 54 AM

@nickofthyme nickofthyme added 7.9.0 backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v7.9.0 and removed 7.9.0 labels Jul 29, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

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

@cchaos cchaos removed their request for review July 29, 2020 16:45
@nickofthyme
Copy link
Contributor Author

@flash1293 or @wylieconlon could yo verify this fix?

Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

LGTM, tested locally and I can now see correctly a clustered bar chart with a dual-axis.

It's just a bit weird that the title says: Stacked bar chart but that seems to be another story: @flash1293 @wylieconlon is that a known bug?
Screenshot 2020-07-30 at 16 31 38

@flash1293
Copy link
Contributor

Stacked bar chart should be stacked indeed, maybe we went too far with the unstacking. I will look into this.

@flash1293
Copy link
Contributor

I checked on current master and it seems this case is working fine there:
Screenshot 2020-07-30 at 17 52 03

@nickofthyme Could you check whether this might be an elastic-charts problem?

@flash1293
Copy link
Contributor

flash1293 commented Jul 30, 2020

Wait, I misunderstood your screenshot, @markov00 - stacking just means stacking the "break down by" dimension you don't have. So while this might be a little confusing, it's still the right behavior when you got two separate axes IMHO. I'll test the PR real quick and approve.

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested a few cases in Chrome and works as expected. Please make sure to get the same update into master (if you haven't done so already).

@nickofthyme
Copy link
Contributor Author

This fix came after a breaking change in @elastic/charts so we are working with eui to upgrade, which will include this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v7.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants