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

gl2d better ticks #1766

Merged
merged 7 commits into from
Jun 9, 2017
Merged

gl2d better ticks #1766

merged 7 commits into from
Jun 9, 2017

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Jun 8, 2017

🆘 this is debatably a breaking change 🆘

In brief, this PR removes a block (see commit 3d67177) which dates back to the first gl2d implementation and recently highlighted in bbac99b (part of the gl2d annotation PR #1301). Back in #1301, I thought that this block was necessary to correctly draw the gl2d axis ticks, but I was wrong. That block was plain wrong from the start. In more technical terms, ax.setScale() sets the correct ax._length, no need to override it using glplot.viewBox.

Removing this block makes the gl2d baselines more on-par with gl2d. To demonstrate, commits d63578b and fbeb5e9 regenerate the scattergl baselines as SVG scatter traces, so comparing (in swipe mode especially) the baselines in commit 32efccd shows little difference between the SVG and gl2d versions. But comparing the new baselines with the old in the combined diff shows some pretty significant differences.

So, what do you think? Can this be considered a bug fix? Or, should we wait for v2?

cc @alexcjohnson @dfcreative @monfera @rreusser

TODO

  • fixup jasmine tests which rely on wrong axis range values

etpinard added 5 commits June 8, 2017 10:11
- to compare output with better gl2d tick patch
- instead of 'old' mesure that uses glplot.viewBox
- this makes gl2d ticks 'more on-par' with SVG
@etpinard
Copy link
Contributor Author

etpinard commented Jun 8, 2017

Full disclosure, I found out that the block removed in 3d67177 was wrong while working on scattergl lasso (PR #1657). As of the latest commit, 1D select boxes have the wrong width (because they have the wrong ax._length):

peek 2017-06-08 10-51

@monfera
Copy link
Contributor

monfera commented Jun 8, 2017

Better unity looks great and on first sight I don't see a negative with it, though my work on gl2d didn't touch on the axis ticks in this regard.

@alexcjohnson
Copy link
Collaborator

Looks great, I'm happy to call this unification a bug fix.

@etpinard etpinard added this to the 1.28.0 milestone Jun 9, 2017
@etpinard etpinard merged commit a6db253 into master Jun 9, 2017
@etpinard etpinard deleted the gl2d-better-ticks branch June 9, 2017 21:12
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