Skip to content

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

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.

Sign up for GitHub

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 4 commits into from
May 23, 2019
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0ca45f0
fix issue 3871 - pie-like legend with line color array
archmoj May 22, 2019
b0b4250
correct funnelarea baseline
archmoj May 22, 2019
c960d0d
keep arraLen 3 as before - add extra condition for color similar to c…
archmoj May 23, 2019
da77d21
revert minExtend and patch line.color in legend style
archmoj May 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,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 •
edited
Loading

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 😏

if(k === 'color' || k === 'colorscale') {
objOut[k] = v.slice();
} else {
objOut[k] = v.slice(0, arrayLen);
Expand Down
Binary file modified test/image/baselines/funnelarea_line_width.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/image/baselines/pie_legend_line_color_array.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
41 changes: 41 additions & 0 deletions test/image/mocks/pie_legend_line_color_array.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"data": [
{
"type": "pie",
"labels": [
"a",
"b",
"c",
"yellow"
],
"values": [
1,
3,
5,
7
],
"marker": {
"colors": [
"pink",
"lightgreen",
"skyblue",
"orange"
],
"line": {
"width": 5,
"color": [
"red",
"green",
"blue",
"yellow"
]
}
}
}
],
"layout": {
"width": 400,
"height": 400,
"title": { "text": "pie-like legend with line.color array of length > 3" }
}
}