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

fix: ignore disabled series in stacked bar values #116

Conversation

etr2460
Copy link
Contributor

@etr2460 etr2460 commented May 31, 2019

🐛 Bug Fix
Sets the bar values to the sum of only enabled series. This means that when you click on the legend to enable/disable series in multibar charts, the bar value will be the sum of only the visible bars. Unable to test right now because the build is broken, but will include screenshots once I can.

See sums changing when series are disabled:
Screen Shot 2019-06-06 at 3 48 44 PM
Screen Shot 2019-06-06 at 3 48 58 PM

to: @kristw

@etr2460 etr2460 requested a review from a team as a code owner May 31, 2019 00:35
@kristw
Copy link
Collaborator

kristw commented Jun 6, 2019

Master is back! Build not broken anymore. Please rebase and push again.

@etr2460 etr2460 force-pushed the erik-ritter--correct-stacked-bar-values-sum branch from a8a96a6 to 7d4fd97 Compare June 6, 2019 17:46
@codecov
Copy link

codecov bot commented Jun 6, 2019

Codecov Report

Merging #116 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #116   +/-   ##
=======================================
  Coverage   19.87%   19.87%           
=======================================
  Files           8        8           
  Lines         161      161           
  Branches       10       10           
=======================================
  Hits           32       32           
  Misses        128      128           
  Partials        1        1
Impacted Files Coverage Δ
.../superset-ui-legacy-preset-chart-nvd3/src/utils.js 6.92% <0%> (ø) ⬆️

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 facd4ca...65c5d47. Read the comment docs.

@etr2460 etr2460 force-pushed the erik-ritter--correct-stacked-bar-values-sum branch 2 times, most recently from 0cac6c1 to 279b7c6 Compare June 6, 2019 22:49
@etr2460
Copy link
Contributor Author

etr2460 commented Jun 6, 2019

I'm also removing some of the test data for nvd3 bar charts since they weren't formatting very well in storybook

@netlify
Copy link

netlify bot commented Jun 6, 2019

Deploy preview for superset-ui-plugins ready!

Built with commit 8f251d0

https://deploy-preview-116--superset-ui-plugins.netlify.com

@etr2460
Copy link
Contributor Author

etr2460 commented Jun 7, 2019

@kristw this should be good for review now

@etr2460
Copy link
Contributor Author

etr2460 commented Jun 7, 2019

I'll need you to merge it too since I don't have write access

@conglei
Copy link
Contributor

conglei commented Jun 7, 2019

The build failed :(

@etr2460
Copy link
Contributor Author

etr2460 commented Jun 8, 2019

They weren’t due to changes in anything I touched though, were they? It looked like lint errors in other files

@kristw
Copy link
Collaborator

kristw commented Jun 8, 2019 via email

@etr2460 etr2460 force-pushed the erik-ritter--correct-stacked-bar-values-sum branch 3 times, most recently from 8f251d0 to f61f7c1 Compare June 10, 2019 19:07
@etr2460 etr2460 force-pushed the erik-ritter--correct-stacked-bar-values-sum branch from f61f7c1 to 6a136b0 Compare June 10, 2019 19:12
@kristw
Copy link
Collaborator

kristw commented Jun 10, 2019

I'll merge and see if it goes away. Travis passes which is more important. Netlify is nice to have

@kristw kristw merged commit da8390e into apache-superset:master Jun 10, 2019
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
* feat: 🎸 Improved QueryObject to handle more fields

The commit is to ensure the feature parity between frontend and backend
QueryOjbect

* test: 💍 Added tests to improve the coverage
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.

3 participants