You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by "Ovilia (via GitHub)" <gi...@apache.org> on 2023/02/02 10:32:37 UTC
[GitHub] [echarts] Ovilia opened a new pull request, #18229: fix(markArea): markArea range in bar series #18130
Ovilia opened a new pull request, #18229:
URL: https://github.com/apache/echarts/pull/18229
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [x] bug fixing
- [ ] new feature
- [ ] others
### What does this PR do?
<!-- USE ONE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
Fix the markArea range problem in bar series.
### Fixed issues
#18130
## Details
### Before: What was the problem?
The markArea range is wrong in bar series when the axisTick is not all displayed. This was a bug introduced in #17098.
### After: How does it behave after the fixing?
`tickValue` is the index in the total axis range, rather than the visible part. After the fixing, it works well when axis has interval or with dataZoom.
## Document Info
One of the following should be checked.
- [x] This PR doesn't relate to document changes
- [ ] The document should be updated later
- [ ] The document changes have been made in apache/echarts-doc#xxx
## Misc
### ZRender Changes
- [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
### Related test cases or examples to use the new APIs
`bar-markArea.html`
## Others
### Merging options
- [ ] Please squash the commits into a single one when merging.
### Other information
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a diff in pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "plainheart (via GitHub)" <gi...@apache.org>.
plainheart commented on code in PR #18229:
URL: https://github.com/apache/echarts/pull/18229#discussion_r1126025637
##########
test/bar-markArea.html:
##########
@@ -168,7 +172,325 @@
});
</script>
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ // This bug only occurs when end is large enough for some of the
+ // axis labels to be hidden due to overlapping.
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main2', {
+ title: [
+ 'There should be a red markArea at x position 20~60 (the position between 60 and 62)',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 50,
+ end: 100
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main3', {
+ title: [
+ 'There should be a red markArea at x position 35~60',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 0,
+ end: 50
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main4', {
+ title: [
+ 'There should be a red markArea at x position 20~35',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 60,
+ end: 60
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main5', {
+ title: [
+ 'There should be a red markArea at the full grid area',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ startValue: 0,
+ endValue: 61
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main6', {
+ title: [
+ 'There should be a red markArea at 20~60. **Note the right should NOT be at the right boundary.**',
Review Comment:
Thanks for your detailed explaination!
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Ovilia commented on a diff in pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "Ovilia (via GitHub)" <gi...@apache.org>.
Ovilia commented on code in PR #18229:
URL: https://github.com/apache/echarts/pull/18229#discussion_r1125991854
##########
src/component/marker/markerHelper.ts:
##########
@@ -105,14 +105,14 @@ export function dataTransform(
const data = seriesModel.getData();
const coordSys = seriesModel.coordinateSystem;
- const dims = coordSys.dimensions;
+ const dims = coordSys?.dimensions;
Review Comment:
Thanks for letting me know about this!
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Ovilia commented on pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "Ovilia (via GitHub)" <gi...@apache.org>.
Ovilia commented on PR #18229:
URL: https://github.com/apache/echarts/pull/18229#issuecomment-1445810220
@plainheart Thanks. I've updated the logic and put that in the test. Please check if anything is missing.
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Ovilia commented on a diff in pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "Ovilia (via GitHub)" <gi...@apache.org>.
Ovilia commented on code in PR #18229:
URL: https://github.com/apache/echarts/pull/18229#discussion_r1126008965
##########
test/bar-markArea.html:
##########
@@ -168,7 +172,325 @@
});
</script>
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ // This bug only occurs when end is large enough for some of the
+ // axis labels to be hidden due to overlapping.
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main2', {
+ title: [
+ 'There should be a red markArea at x position 20~60 (the position between 60 and 62)',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 50,
+ end: 100
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main3', {
+ title: [
+ 'There should be a red markArea at x position 35~60',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 0,
+ end: 50
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main4', {
+ title: [
+ 'There should be a red markArea at x position 20~35',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 60,
+ end: 60
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main5', {
+ title: [
+ 'There should be a red markArea at the full grid area',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ startValue: 0,
+ endValue: 61
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main6', {
+ title: [
+ 'There should be a red markArea at 20~60. **Note the right should NOT be at the right boundary.**',
Review Comment:
1. I didn't change the logic of axis label. The logic of the alignment is that, when overlap, some ticks and labels are hidden, but the rest labels don't change position. For example, in the image below, the red tick is hidden because of overlapping, and the label `2` is aligned center according to the blue arrow range rather than orange arrow.
<img width="744" alt="image" src="https://user-images.githubusercontent.com/779050/223040742-37421657-e2d4-46c9-9824-11d7f768d703.png">
2. The following blue and green bars are at the x position of `2`. Similarly, the bar at the x position of `60` is as the yellow bar shows.
<img width="419" alt="image" src="https://user-images.githubusercontent.com/779050/223039836-65389bce-e121-4ac2-9f29-5096f6fd4372.png">
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Ovilia commented on a diff in pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "Ovilia (via GitHub)" <gi...@apache.org>.
Ovilia commented on code in PR #18229:
URL: https://github.com/apache/echarts/pull/18229#discussion_r1125996001
##########
test/bar-markArea.html:
##########
@@ -168,7 +172,325 @@
});
</script>
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ // This bug only occurs when end is large enough for some of the
+ // axis labels to be hidden due to overlapping.
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main2', {
+ title: [
+ 'There should be a red markArea at x position 20~60 (the position between 60 and 62)',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 50,
+ end: 100
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main3', {
+ title: [
+ 'There should be a red markArea at x position 35~60',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 0,
+ end: 50
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main4', {
+ title: [
+ 'There should be a red markArea at x position 20~35',
Review Comment:
Same as above
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Ovilia merged pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "Ovilia (via GitHub)" <gi...@apache.org>.
Ovilia merged PR #18229:
URL: https://github.com/apache/echarts/pull/18229
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] echarts-bot[bot] commented on pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "echarts-bot[bot] (via GitHub)" <gi...@apache.org>.
echarts-bot[bot] commented on PR #18229:
URL: https://github.com/apache/echarts/pull/18229#issuecomment-1413517288
Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/echarts/wiki/How-to-make-a-pull-request).
The pull request is marked to be `PR: author is committer` because you are a committer of this project.
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a diff in pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "plainheart (via GitHub)" <gi...@apache.org>.
plainheart commented on code in PR #18229:
URL: https://github.com/apache/echarts/pull/18229#discussion_r1125851298
##########
src/component/marker/markerHelper.ts:
##########
@@ -105,14 +105,14 @@ export function dataTransform(
const data = seriesModel.getData();
const coordSys = seriesModel.coordinateSystem;
- const dims = coordSys.dimensions;
+ const dims = coordSys?.dimensions;
Review Comment:
Using the optional chain operator might bring some redundant code when compiling to a lower ES version that doesn't support it. For example, the above code may be compiled as,
```js
var dims = coordSys === null || coordSys === void 0 ? void 0 : coordSys.dimensions;
```
Compared to `&&`, it doesn't look neat.
##########
test/bar-markArea.html:
##########
@@ -168,7 +172,325 @@
});
</script>
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ // This bug only occurs when end is large enough for some of the
+ // axis labels to be hidden due to overlapping.
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main2', {
+ title: [
+ 'There should be a red markArea at x position 20~60 (the position between 60 and 62)',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 50,
+ end: 100
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main3', {
+ title: [
+ 'There should be a red markArea at x position 35~60',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 0,
+ end: 50
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main4', {
+ title: [
+ 'There should be a red markArea at x position 20~35',
Review Comment:
The number range `20~35` in the title doesn't match the one in the above option.
##########
test/bar-markArea.html:
##########
@@ -168,7 +172,325 @@
});
</script>
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ // This bug only occurs when end is large enough for some of the
+ // axis labels to be hidden due to overlapping.
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main2', {
+ title: [
+ 'There should be a red markArea at x position 20~60 (the position between 60 and 62)',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 50,
+ end: 100
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main3', {
+ title: [
+ 'There should be a red markArea at x position 35~60',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 0,
+ end: 50
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main4', {
+ title: [
+ 'There should be a red markArea at x position 20~35',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 60,
+ end: 60
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main5', {
+ title: [
+ 'There should be a red markArea at the full grid area',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ startValue: 0,
+ endValue: 61
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main6', {
+ title: [
+ 'There should be a red markArea at 20~60. **Note the right should NOT be at the right boundary.**',
Review Comment:
I'm not sure if the last case is expected.
<img src="https://user-images.githubusercontent.com/26999792/223015290-2f45d297-10f9-4de7-bab1-ebcac1da21f1.png" width="400">
1. Why the labels are neither aligned **_center_** nor aligned with ticks?
2. The red markArea seems **_over 60_**?
##########
test/bar-markArea.html:
##########
@@ -168,7 +172,325 @@
});
</script>
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ // This bug only occurs when end is large enough for some of the
+ // axis labels to be hidden due to overlapping.
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main2', {
+ title: [
+ 'There should be a red markArea at x position 20~60 (the position between 60 and 62)',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 50,
+ end: 100
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main3', {
+ title: [
+ 'There should be a red markArea at x position 35~60',
Review Comment:
The number range `35~60` in the title doesn't match the one in the above option.
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] echarts-bot[bot] commented on pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "echarts-bot[bot] (via GitHub)" <gi...@apache.org>.
echarts-bot[bot] commented on PR #18229:
URL: https://github.com/apache/echarts/pull/18229#issuecomment-1455675910
Congratulations! Your PR has been merged. Thanks for your contribution! 👍
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] ily-salamat commented on pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "ily-salamat (via GitHub)" <gi...@apache.org>.
ily-salamat commented on PR #18229:
URL: https://github.com/apache/echarts/pull/18229#issuecomment-1505175830
Hi @Ovilia, I opened a new issue here #18499 related to this PR, could you check it please and thanks for your efforts
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart commented on a diff in pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "plainheart (via GitHub)" <gi...@apache.org>.
plainheart commented on code in PR #18229:
URL: https://github.com/apache/echarts/pull/18229#discussion_r1094452711
##########
src/chart/bar/BaseBarSeries.ts:
##########
@@ -94,20 +94,77 @@ class BaseBarSeriesModel<Opts extends BaseBarSeriesOption<unknown> = BaseBarSeri
const coordSys = this.coordinateSystem;
if (coordSys && coordSys.clampData) {
// PENDING if clamp ?
- const pt = coordSys.dataToPoint(coordSys.clampData(value));
+ const clampData = coordSys.clampData(value);
+ const pt = coordSys.dataToPoint(clampData);
if (startingAtTick) {
each(coordSys.getAxes(), function (axis: Axis2D, idx: number) {
// If axis type is category, use tick coords instead
if (axis.type === 'category') {
const tickCoords = axis.getTicksCoords();
- let tickIdx = coordSys.clampData(value)[idx];
+
+ let targetTickId = clampData[idx];
// The index of rightmost tick of markArea is 1 larger than x1/y1 index
- if (dims && (dims[idx] === 'x1' || dims[idx] === 'y1')) {
- tickIdx += 1;
+ const isEnd = dims[idx] === 'x1' || dims[idx] === 'y1';
+ if (dims && isEnd) {
+ targetTickId += 1;
+ }
Review Comment:
Similar to the issue mentioned in the previous comment, `dims` may be not present.
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] Ovilia commented on a diff in pull request #18229: fix(markArea): markArea range in bar series #18130
Posted by "Ovilia (via GitHub)" <gi...@apache.org>.
Ovilia commented on code in PR #18229:
URL: https://github.com/apache/echarts/pull/18229#discussion_r1125995505
##########
test/bar-markArea.html:
##########
@@ -168,7 +172,325 @@
});
</script>
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ // This bug only occurs when end is large enough for some of the
+ // axis labels to be hidden due to overlapping.
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main2', {
+ title: [
+ 'There should be a red markArea at x position 20~60 (the position between 60 and 62)',
+ 'Fix #18130'
+ ],
+ option: option
+ });
+ });
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ ], function (echarts) {
+ var option;
+ var end = 70;
+
+ var xData = [];
+ for (let i = 0; i < end; ++i) {
+ xData.push(i);
+ }
+
+ option = {
+ xAxis: [
+ {
+ data: xData,
+ show: true
+ }
+ ],
+ yAxis: [{ type: 'value', show: true, max: 100 }],
+ grid: [{}],
+ dataZoom: {
+ type: 'slider',
+ xAxisIndex: 0,
+ start: 50,
+ end: 100
+ },
+ series: [
+ {
+ type: 'bar',
+ data: [[end - 1, 100, 22]],
+ barWidth: 10,
+ itemStyle: {
+ opacity: 0.5
+ },
+ markArea: {
+ itemStyle: {
+ color: 'red',
+ opacity: 0.5
+ },
+ data: [
+ [
+ {
+ xAxis: 20
+ },
+ {
+ xAxis: end - 10
+ }
+ ]
+ ]
+ }
+ }
+ ]
+ };
+ var chart = testHelper.create(echarts, 'main3', {
+ title: [
+ 'There should be a red markArea at x position 35~60',
Review Comment:
The dataZoom visible area is starting from 35 (`dataZoom.start` is 50% and the range of the xAxis is 0~70), so this `35~60` is expected.
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org