-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
[New Viz] Nightingale Rose Chart #3676
[New Viz] Nightingale Rose Chart #3676
Conversation
700defc
to
4b7789f
Compare
Coverage increased (+0.8%) to 71.135% when pulling 4b7789f204055518bac067dd23e3b5b558ab8669 on Mogball:mogball/feature/rose_chart into d0b5b44 on apache:master. |
4b7789f
to
e06214b
Compare
Coverage increased (+0.8%) to 71.147% when pulling aa9d96b17f05bdb2e63e933d0af7e2165c866de1 on Mogball:mogball/feature/rose_chart into 2a89c90 on apache:master. |
3 similar comments
Coverage increased (+0.8%) to 71.147% when pulling aa9d96b17f05bdb2e63e933d0af7e2165c866de1 on Mogball:mogball/feature/rose_chart into 2a89c90 on apache:master. |
Coverage increased (+0.8%) to 71.147% when pulling aa9d96b17f05bdb2e63e933d0af7e2165c866de1 on Mogball:mogball/feature/rose_chart into 2a89c90 on apache:master. |
Coverage increased (+0.8%) to 71.147% when pulling aa9d96b17f05bdb2e63e933d0af7e2165c866de1 on Mogball:mogball/feature/rose_chart into 2a89c90 on apache:master. |
aa9d96b
to
e06214b
Compare
b8dbe4f
to
38ecc7c
Compare
1 similar comment
@@ -0,0 +1,540 @@ | |||
/* eslint no-use-before-define: ["error", { "functions": false }] */ |
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.
This should probably just be an js file instead of jsx
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.
👍
let inTransition = false; | ||
const ae = roseWrap | ||
.selectAll('g') | ||
.data(JSON.parse(JSON.stringify(arcSt.data))) // deep copy data state |
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.
why does this need to be deep copied?
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.
The object passed to .data()
becomes the 'state' of the arcs and is modified when they transition. arcSt.data
needs to be copied so that the arcs can transition back to their original position.
endAngle: nArcSt.pie[segments[0] + d.arcId].endAngle, | ||
})(d)) | ||
.style('opacity', d => | ||
state.disabled[d.arcId] || arcSt.pie[segments[0] + d.arcId].percent < labelThreshold ? |
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.
I like parenthesis for these kinds of expressions.
d1920a7
to
9c434bf
Compare
9c434bf
to
d507380
Compare
@mistercrunch I would also like to bump this one |
@Mogball can you fix the merge conflicts? |
* Nightingale Rose Chart * Review comments
* Nightingale Rose Chart * Review comments
Also known as a Polar Area Diagram, the Nightingale Rose Chart displays time series data on a polar coordinate system. The Rose Chart is meant to visualize periodic data (i.e. daily, weekly, monthly) to show variation over that cycle.
Features