add white boundary for the donut marker #116
Conversation
| @@ -215,11 +215,18 @@ function donutMarkerSVGIcon(props: DonutMarkerStandaloneProps): { | |||
| } { | |||
| const scale = props.markerScale ?? MarkerScaleDefault; | |||
| const size = 40 * scale; | |||
| // set outter white circle size to describe white boundary | |||
| const outerWhiteCircleSize = size / 2 + size / 16; | |||
There was a problem hiding this comment.
Can we call this backgroundWhiteCircleRadius? The 'background' because we see it's outer and inner parts since it's behind the data donut, and 'radius' because it looks like we usually use 'size' as a diameter but below in line 252 we're using it as a radius.
There was a problem hiding this comment.
@asizemore Thank you for your reviews, tests, and suggestions! 👍 I like the word so will change it accordingly.
| @@ -266,31 +273,32 @@ function donutMarkerSVGIcon(props: DonutMarkerStandaloneProps): { | |||
| // only sum up in cumulative mode | |||
| cumulativeSum += thisValue; | |||
|
|
|||
| //DKDK draw arc: makeArc(centerX, centerY, Radius for arc, start point of arc (radian), end point of arc (radian)) | |||
| // draw arc: makeArc(centerX, centerY, Radius for arc, start point of arc (radian), end point of arc (radian)) | |||
| svgHTML += | |||
| '<path fill="none" stroke="' + | |||
| (el.color ?? 'silver') + | |||
| '" stroke-width="4" d="' + | |||
There was a problem hiding this comment.
What do you think about setting stroke-width to be 5? It was kinda nice to see more of the data colors, but perhaps there was a reason to set it to 4 originally?
(with stroke-width=5)
There was a problem hiding this comment.
@asizemore I don't think we have a specific reason not to use larger stroke-width as you suggested. 🤔 Will ask Bob about this at slack, just in case.
There was a problem hiding this comment.
I like the bolder look. The only issue is squeezing in the numbers. I think above 9999 we switch to 10k, 11k etc, so we should be fine up to 999k. Looks like room for 4 characters?
|
@bobular yes I think the number label would be fine. I will change to 5 as @asizemore suggested. Thank you for sharing your insight 👍 |
This will resolve VEuPathDB/web-components#345. Here is a screenshot: