100pah commented on code in PR #20184:
URL: https://github.com/apache/echarts/pull/20184#discussion_r1828098251
##########
test/bar-polar-basic-radial.html:
##########
@@ -72,5 +86,74 @@
window.onresize = chart.resize;
});
</script>
+
+
+
+ <script>
+
+ require(['echarts'], function (echarts) {
+
+ var chart = echarts.init(document.getElementById('main2'));
+
+ var config = {
+ startAngle: 90,
+ endAngle: -270,
+ clockwise: true,
+ testNext: () => {
+ config.startAngle = 90;
+ config.endAngle = -90;
Review Comment:
Just a tiny problem. when clicking on 'testNext', the 'endAngle' in dat.GUI
isn't updated, which caused confusion a little bit.
(It's OK not to fix it.)
##########
src/coord/polar/polarCreator.ts:
##########
@@ -102,9 +102,15 @@ function updatePolarScale(this: Polar, ecModel:
GlobalModel, api: ExtensionAPI)
// Fix extent of category angle axis
if (angleAxis.type === 'category' && !angleAxis.onBand) {
const extent = angleAxis.getExtent();
- const diff = 360 / (angleAxis.scale as OrdinalScale).count();
- angleAxis.inverse ? (extent[1] += diff) : (extent[1] -= diff);
- angleAxis.setExtent(extent[0], extent[1]);
+ const angleModel = angleAxis.model;
+ const endAngle = angleModel.get('endAngle');
+ const spanAngle = (endAngle == null ? 360 : endAngle -
angleModel.get('startAngle'))
Review Comment:
The conditional operator has lower precedence than arithmetic operator.
`endAngle == null ? 360 : endAngle - angleModel.get('startAngle')`
means
`endAngle == null ? 360 : (endAngle - angleModel.get('startAngle'))`
This should be a "famous" and commonly occurring mistake in not only JS but
also Java, c++, ...
And why there is null checker for only `endAngle` but not `startAngle`?
I think in this place we can treat that both `endAngle` and `startAngle` are
not nullable here. I mean that set default value in only one place (function
`setAxis`).
And should we retrieve value from `extent` rather from angleModel here?
##########
src/coord/polar/polarCreator.ts:
##########
@@ -102,9 +102,15 @@ function updatePolarScale(this: Polar, ecModel:
GlobalModel, api: ExtensionAPI)
// Fix extent of category angle axis
if (angleAxis.type === 'category' && !angleAxis.onBand) {
const extent = angleAxis.getExtent();
- const diff = 360 / (angleAxis.scale as OrdinalScale).count();
- angleAxis.inverse ? (extent[1] += diff) : (extent[1] -= diff);
- angleAxis.setExtent(extent[0], extent[1]);
+ const angleModel = angleAxis.model;
+ const endAngle = angleModel.get('endAngle');
+ const spanAngle = (endAngle == null ? 360 : endAngle -
angleModel.get('startAngle'))
+ * (angleAxis.inverse ? -1 : 1);
+ const diff = spanAngle / (angleAxis.scale as OrdinalScale).count();
+ if (Math.abs(spanAngle + diff) >= 360) {
+ extent[1] += Math.abs(diff);
+ angleAxis.setExtent(extent[0], extent[1]);
Review Comment:
In the current modified approach, this cases are not reasonable enougth:
<img width="983" alt="image"
src="https://github.com/user-attachments/assets/3a6c182d-8de9-4fbb-b0d1-62b026e10065";>
(already have a overlap between bar C and bar D)
<img width="987" alt="image"
src="https://github.com/user-attachments/assets/18924196-5cff-4666-8bf9-f7e62c56d647";>
(endAngle changes a little (from -195 to -198) but the appearance changes
significantly.)
I think the "adjusting the extent" is a compromise. It's counter intuitive
(the adjusted endAngle is not what use specified) but with out that user have
avoid overlap manually.
I just try to modify it: make it keep at a "max span" when there is no
enough space.
```js
// Fix extent of category angle axis
if (angleAxis.type === 'category' && !angleAxis.onBand) {
const extent = angleAxis.getExtent();
let spanAngle = normalizeAngle((normalizeAngle(extent[1]) + 360 -
normalizeAngle(extent[0])));
if (angleAxis.inverse) {
spanAngle = 360 - spanAngle;
}
const spanLimit = 360 - 360 / (angleAxis.scale as
OrdinalScale).count();
if (spanAngle >= spanLimit) {
extent[1] = extent[0] + (angleAxis.inverse ? -1 : 1) * spanLimit;
angleAxis.setExtent(extent[0], extent[1]);
}
}
// FIXME: this kind of function should be placed in some common util
file? (or similar one already exists?)
// Normalize an angle value to `[0, 360)`.
function normalizeAngle(val: number) {
val = val % 360;
val < 0 && (val += 360);
return val;
}
```
(it's just a illustrative code to show my present understanding.)
But about clockwise (inverse). I haven't understood the logic yet.
The current behavior (before this PR modified) seems weird:
<img width="1046" alt="image"
src="https://github.com/user-attachments/assets/18454e8c-f6a8-49d1-a59c-43e348a7204e";>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
