Skip to content

Bar: fix issue #372 - bar widths with nonnumeric sizes #542

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

Merged
merged 1 commit into from
May 17, 2016

Conversation

seibs
Copy link
Contributor

@seibs seibs commented May 16, 2016

PR for seibs#1
Fixes issue #372

Bars were being filtered out too soon when they had a non-numeric size. This change leaves them in so the bar width/placement calculations take them into account, and leaves filtering them out to the logic already in src/traces/bar/plot.js.

@etpinard etpinard added bug something broken status: in progress labels May 17, 2016
@etpinard
Copy link
Contributor

Great.

As mentioned in #539, your patch also fixes a very old bug that made it through our image test mocks.

Before merging this PR, you'll have to update the benchmark.png image baseline.

To do so, (1) go to https://circleci.com/gh/plotly/plotly.js/1457 and click on the Artifact tab:

image

(2) navigate to the test_images folder:

image

(3) download the benchmark.png image and (4) finally replace test/image/baselines/benchmarks.png with it

@seibs
Copy link
Contributor Author

seibs commented May 17, 2016

Whoops, missed you're comment in #539.

The only tabs visible on circleci were "Test Summary", "circle.yml", and "Build Timing" so I generated the png following the directions in the test/image/README.md.

@n-riesco
Copy link
Contributor

n-riesco commented May 17, 2016 via email

@etpinard
Copy link
Contributor

@seibs Thanks! You get my vote for PR of the week 🏆 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants