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

Scatterpolargl fixes #3098

Merged
merged 8 commits into from Oct 24, 2018

Conversation

Projects
None yet
2 participants
@etpinard
Copy link
Member

commented Oct 11, 2018

fixes #3094 and brings scatterpolargl axis expansion on-par with scatterpolar.

cc @plotly/plotly_js

etpinard added some commits Oct 12, 2018

no need to set 'stash.count'
... just use trace._length instead (which is far less confusing).
compute scatterpolargl style opts during calc
- and use same TOO_MANY_POINTS axis expansion logic as scattergl
- stash style opts and extend them with position opts
  during plot.
@etpinard

This comment has been minimized.

Copy link
Member Author

commented Oct 24, 2018

Ready for review again.

@@ -129,13 +126,10 @@ function calc(gd, trace) {

// stash scene ref
stash._scene = scene;
stash.index = scene.count;
stash.index = scene.count++;

This comment has been minimized.

Copy link
@alexcjohnson

alexcjohnson Oct 24, 2018

Contributor

this of course works... but I can't say I like using ++ mid-assignment

function calc(container, trace) {
var layout = container._fullLayout;
function calc(gd, trace) {
var fullLayout = gd._fullLayout;

This comment has been minimized.

Copy link
@alexcjohnson

alexcjohnson Oct 24, 2018

Contributor

ha yeah, that's certainly clearer! 🍰

@alexcjohnson

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

Beautiful! With the slight exception of #3098 (comment) but I won't hold up the 💃 for that.
Love the new trace type test.

@etpinard etpinard merged commit c10eb6f into master Oct 24, 2018

7 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: publish Your tests passed on CircleCI!
Details
ci/circleci: test-image Your tests passed on CircleCI!
Details
ci/circleci: test-image2 Your tests passed on CircleCI!
Details
ci/circleci: test-jasmine Your tests passed on CircleCI!
Details
ci/circleci: test-jasmine2 Your tests passed on CircleCI!
Details
ci/circleci: test-syntax Your tests passed on CircleCI!
Details

@etpinard etpinard deleted the scatterpolargl-2-scatterpolar branch Oct 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.