Skip to content

Bar chart with second y axis overlaps data series#4150

Merged
kravets-levko merged 4 commits into
masterfrom
fix/bar-chart-2292
Aug 5, 2020
Merged

Bar chart with second y axis overlaps data series#4150
kravets-levko merged 4 commits into
masterfrom
fix/bar-chart-2292

Conversation

@kravets-levko

@kravets-levko kravets-levko commented Sep 18, 2019

Copy link
Copy Markdown
Collaborator

What type of PR is this? (check all applicable)

  • Bug Fix

Description

Assign offsetgroup for bar series so Plotly will offset them properly.

Related Tickets & Documents

Fixes #2292

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Before:
image

After (the same chart settings and data, but with offsetgroup):
image

@kravets-levko
kravets-levko marked this pull request as ready for review September 18, 2019 13:45

@ranbena ranbena left a comment

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.

Noticed in demo that some of the widget visualizations changed.
For instance, Group Scatter (x axis labels hidden), Labels (same), Basic Heatmap (blocks now distinguishable which is good?), Basic Bubble (axis offset).

@ranbena

ranbena commented Sep 18, 2019

Copy link
Copy Markdown
Contributor

+556 −1,708 😲😛

@kravets-levko kravets-levko changed the title Bar chart with second y axis overlaps data series (WIP) Bar chart with second y axis overlaps data series Sep 18, 2019
@kravets-levko

Copy link
Copy Markdown
Collaborator Author

Noticed in demo that some of the widget visualizations changed.

Yeah, I suppose it's because of plotly.js upgrade. Need to test it properly before merging.

+556 −1,708

That's lockfile

@kravets-levko

Copy link
Copy Markdown
Collaborator Author

Group Scatter (x axis labels hidden), Labels (same)

I see x-axis labels there:

image
image

Bubble axis offset - yeah, that's quite interesting, considering that on both https://deploy-preview-4150--redash-preview.netlify.com/dashboard/demo and https://redash-preview.netlify.com/dashboard/demo it looks wrong 🤔

image
image

Heatmap difference may be related to my recent refactor, I'll check.

Also, seems there is a very subtle difference in plot inner paddings - they are a bit larger on this branch 🤔

@ranbena

ranbena commented Sep 19, 2019

Copy link
Copy Markdown
Contributor

I think it has to do with my screen resolution (width 1400px) so it now looks like this:

Screen Shot 2019-09-19 at 10 49 51

Screen Shot 2019-09-19 at 10 49 23

Also, seems there is a very subtle difference in plot inner paddings

I noticed it but don't mind it. Not a dramatic change.

@kravets-levko

Copy link
Copy Markdown
Collaborator Author

@gabrieldutra @arikfr (@ranbena if you're still interested in this 😄) I updated this PR and now it contains only changes related to the bug it's fixing (no more Plotly upgrade - we did it separately). I kindly ask you to review it once more 🙇‍♂️

@arikfr If v9 will contain upgraded Plotly - maybe it makes some sense to include this fix as well? WDYT?

@kravets-levko kravets-levko changed the title (WIP) Bar chart with second y axis overlaps data series Bar chart with second y axis overlaps data series Apr 7, 2020
@orenpai

orenpai commented May 5, 2020

Copy link
Copy Markdown

+1 for pulling this one in, I'm affected by this issue too. Thanks for your contribution!

@kravets-levko
kravets-levko merged commit eb603f6 into master Aug 5, 2020
@kravets-levko
kravets-levko deleted the fix/bar-chart-2292 branch August 5, 2020 17:28
andrewdever pushed a commit to andrewdever/redash that referenced this pull request Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bar chart with second y axis overlaps data series

4 participants