-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Pie legend and showlegend per slice #7580
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
Conversation
…ice legend visibility.
…legend assignment.
…pies with showlegend and/or legend as arrays
|
@alexshoe Hi Alex! Do you possibly have an estimate when this PR could be tackled? Also, let me know if you have any questions :) |
|
Hey @my-tien! Sorry, just working through a backlog of different PRs that I was assigned. I'll start reviewing this tomorrow. |
…isibility # Conflicts: # test/plot-schema.json
|
@alexshoe Hi Alex, I saw that my branch got out of date and conflicted with the current EDIT: After wiping node_modules and re-installing with
|
|
@my-tien could you please add some more information to the OP? I'm interested in seeing sections describing:
With that, I'll be able to better review the changes. |
…d instead of labels.
|
Hi @camdecoster, I also updated the OP and added a better description of the properties for the plot schema. Please let me know if anything is unclear or missing! |
camdecoster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this feature! There are a few changes to make, but it seems to work well.
camdecoster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are a few more suggestions.
src/lib/coerce.js
Outdated
| return value === true || value === false; | ||
| } | ||
|
|
||
| if (opts.arrayOk && isArrayOrTypedArray(v) && v.length > 0 && v.every(isBoolean)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you anticipate typed arrays being used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. So the general approach is not to accept typed arrays everywhere a normal array is accepted? Should I just check for Array.isArray?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If no typed arrays are anticipated, then I think you should just use Array.isArray.
coerce: Simplify test for isSubplotId Co-authored-by: Cameron DeCoster <[email protected]>
camdecoster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for adding this feature.
This PR makes it possible to pass arrays to the pie properties 'legend' and 'showlegend' so that legend assignment and visibility can be configured per slice.
Changes:
Previously,
traces.pie.legendandtraces.pie.showlegendonly accepted one value that applied for all slices. With this PR, slice legend entries can be grouped and individual slices can be hidden in the legend:Result
Old behavior
No possibility to group slices semantically or to hide a slice. Also takes up more vertical space.

Testing
For testing, I added 3 mocks:
label0anddlabelare set instead oflabels.Note:
This feature is not expected to work when pie
valuesare not set.Disclaimer
I am required to add that…the software is provided "as is", without warranty of any kind, express or implied, including but not limited to the warranties of merchantability, fitness for a particular purpose and noninfringement. in no event shall the authors or copyright holders be liable for any claim, damages or other liability, whether in an action of contract, tort or otherwise, arising from, out of or in connection with the software or the use or other dealings in the software.