fix(gauge): y origin can be equal to radius #486
Conversation
Codecov Report
@@ Coverage Diff @@
## main #486 +/- ##
==========================================
- Coverage 85.68% 85.67% -0.02%
==========================================
Files 749 749
Lines 15321 15331 +10
Branches 1814 1816 +2
==========================================
+ Hits 13128 13135 +7
- Misses 2162 2165 +3
Partials 31 31
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| private static readonly GAUGE_ARC_CORNER_RADIUS: number = 10; | ||
| private static readonly GAUGE_AXIS_PADDING: number = 30; | ||
| private static readonly GAUGE_MIN_RADIUS_TO_SHOW_PATH: number = 80; | ||
| private static readonly EXTRA_ARC_ANGLE: number = Math.PI / 12; |
There was a problem hiding this comment.
Is the extra arc angle added to start and end so that the label appears more inside? Instead of this, could we have done this by updating the position of text on line 30?
| startAngle: -Math.PI / 2, | ||
| endAngle: -Math.PI / 2 + (inputData.value / inputData.maxValue) * Math.PI | ||
| startAngle: -Math.PI / 2 - GaugeComponent.EXTRA_ARC_ANGLE, | ||
| endAngle: -Math.PI / 2 - GaugeComponent.EXTRA_ARC_ANGLE + (inputData.value / inputData.maxValue) * Math.PI |
There was a problem hiding this comment.
This seems incorrect to me.
case 1: inputData.value = 0, the end angle would be -Math.PI / 2 - GaugeComponent.EXTRA_ARC_ANGLE which is same as start angle. So the arc will be completely empty. Expected outcome.
case 2: inputData.value = inputData.maxValue, in this case the end angle of value arc should be equal to max end angle of background arc, that is Math.PI / 2 + GaugeComponent.EXTRA_ARC_ANGLE. Line 111, would make it -Math.PI / 2 - GaugeComponent.EXTRA_ARC_ANGLE + 1 * Math.PI = Math.PI / 2 - GaugeComponent.EXTRA_ARC_ANGLE
So the value arc would never be completely full.
There was a problem hiding this comment.
Good catch, I will fix that
| * needs to be calculated using the provided height, instead of width / 2 | ||
| */ | ||
| const radius = width / 2; | ||
| const extraArcHeight = this.calculateExtraArcLength(radius); |
There was a problem hiding this comment.
If we adjust the text position, could we avoid all this additional logic due to the extra arc length?
Description
Math.PI / 12at the bottom of the gauge component, instead of keeping it as semicircle. Adding the extra arc to the semicircle will require extra padding at the bottom, but it cannot be a fixed padding, because the larger the radius, the greater the padding required at the bottom. Approximating extra arc length as the extra padding required at the bottomTesting
Tested with various width and height for the gauge component. It works beautifully auto computing the extra space required below
Smaller radius
Larger radius
Checklist: