Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

fix: responsive y-axis on stacked charts #141

Merged
merged 1 commit into from
Jul 22, 2019

Conversation

etr2460
Copy link
Contributor

@etr2460 etr2460 commented Jul 12, 2019

🐛 Bug Fix

It looks like #45 introduced a new bug where when unselecting series on stacked charts the y-axis wouldn't be responsive to fix the chart size. I've tested this fix with series selected and unselected, no yAxisBounds and yAxisBounded, and everything seems to work as intended.

However, it seems like this logic has been changed a lot lately, so I might be missing edge cases. Let me know if I should test anything else!

No y axis bounds:
Screen Shot 2019-07-12 at 2 32 01 PM
Screen Shot 2019-07-12 at 2 32 09 PM

y axis bounds:
Screen Shot 2019-07-12 at 2 32 20 PM
Screen Shot 2019-07-12 at 2 32 31 PM

cc: @williaster @khtruong @michellethomas @graceguo-supercat

@etr2460 etr2460 requested a review from a team as a code owner July 12, 2019 21:50
@etr2460 etr2460 force-pushed the erik-ritter--fix-stacked-charts branch from 3265724 to 9f42928 Compare July 12, 2019 22:02
@etr2460
Copy link
Contributor Author

etr2460 commented Jul 12, 2019

This also fixes an issue with charts with controls shown.

Before:
Screen Shot 2019-07-12 at 3 05 13 PM

After:
Screen Shot 2019-07-12 at 3 02 59 PM

@etr2460 etr2460 force-pushed the erik-ritter--fix-stacked-charts branch from 9f42928 to aae3367 Compare July 12, 2019 22:15
@etr2460 etr2460 force-pushed the erik-ritter--fix-stacked-charts branch from aae3367 to ef3f562 Compare July 12, 2019 22:20
@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

Merging #141 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #141   +/-   ##
=======================================
  Coverage   19.87%   19.87%           
=======================================
  Files           8        8           
  Lines         161      161           
  Branches       10       10           
=======================================
  Hits           32       32           
  Misses        128      128           
  Partials        1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd383df...ef3f562. Read the comment docs.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 12, 2019
@etr2460
Copy link
Contributor Author

etr2460 commented Jul 12, 2019

Also also fixes responsiveness for line charts:

Before:
Screen Shot 2019-07-12 at 3 43 59 PM

After:
Screen Shot 2019-07-12 at 3 43 36 PM

@etr2460
Copy link
Contributor Author

etr2460 commented Jul 22, 2019

mind taking a look?

@williaster @khtruong @michellethomas @graceguo-supercat

Copy link
Contributor

@khtruong khtruong left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@etr2460
Copy link
Contributor Author

etr2460 commented Jul 22, 2019

@kristw too if you wanna take a look

@etr2460 etr2460 merged commit 8f60a5d into master Jul 22, 2019
@delete-merged-branch delete-merged-branch bot deleted the erik-ritter--fix-stacked-charts branch July 22, 2019 20:53
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
* fix: callapi unit test

* test: fix async calls

* fix: remaining test

* refactor: do not declare unused var
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants