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

fix pie-like legend when line color array has length greater than 3 #3891

Merged
merged 4 commits into from
May 23, 2019

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented May 22, 2019

Fix #3871.

@plotly/plotly_js

@archmoj archmoj added bug something broken status: reviewable labels May 22, 2019
src/lib/index.js Outdated
@@ -644,11 +643,7 @@ lib.minExtend = function(obj1, obj2) {
if(k.charAt(0) === '_' || typeof v === 'function') continue;
else if(k === 'module') objOut[k] = v;
else if(Array.isArray(v)) {
if(k === 'colorscale') {
Copy link
Contributor

@etpinard etpinard May 23, 2019

Choose a reason for hiding this comment

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

Hmm. This will probably slow down performance in some cases.

Can we make try adding another special (similar to k === 'colorscale') instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or perhaps a third arg to the function, to override arrayLen = 3, that gets invoked only when needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or perhaps a third arg to the function, to override arrayLen = 3, that gets invoked only when needed?

Good call - that would work!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revised in c960d0d.

Copy link
Contributor

Choose a reason for hiding this comment

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

As Lib.minExtend is used in the toolpanel code, I would prefer if we would try to fix this bug without patching Lib.minExtend.

Sorry for the confusion @archmoj

Copy link
Collaborator

Choose a reason for hiding this comment

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

a third arg would be fine with toolpanel. I wouldn't want to add an exception like color because in some contexts that can be enormous. I would much prefer to pick out pie traces explicitly and extend the full length in that case, since we wouldn't be bothered by poor perf if someone gives us a pie with 100K slices 😏

@etpinard
Copy link
Contributor

nicely done 💃

@archmoj archmoj merged commit c23b69a into master May 23, 2019
@archmoj archmoj deleted the fix3871-pie-legend-color branch May 23, 2019 17:07
archmoj added a commit that referenced this pull request Jun 5, 2023
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.

Pie legend color is not correct after the third one
3 participants