You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2021/02/03 15:04:02 UTC
[GitHub] [echarts] susiwen8 opened a new pull request #14202: Fix(markline): markline label should have higher z
susiwen8 opened a new pull request #14202:
URL: https://github.com/apache/echarts/pull/14202
<!-- 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?
`Label` should have higher `z index` compare to `line`
### Fixed issues
Close #14190
## Details
### Before: What was the problem?
<img width="472" alt="Screen Shot 2021-02-03 at 23 00 01" src="https://user-images.githubusercontent.com/20318608/106765267-94e15680-6673-11eb-986e-5369e4918778.png">
### After: How is it fixed in this PR?
<img width="467" alt="Screen Shot 2021-02-03 at 22 59 45" src="https://user-images.githubusercontent.com/20318608/106765298-9b6fce00-6673-11eb-8537-9cf0aff08f1d.png">
## Usage
### Are there any API changes?
- [ ] The API has been changed.
<!-- LIST THE API CHANGES HERE -->
### Related test cases or examples to use the new APIs
/test/markline.html
## Others
### Merging options
- [ ] Please squash the commits into a single one when merge.
### 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.
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] susiwen8 commented on pull request #14202: Fix(markline): markline label should have higher z
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #14202:
URL: https://github.com/apache/echarts/pull/14202#issuecomment-773318062
@pissang It seems `markline` is a `Group` for which `z` for `textContent` has been marked as `TODO`, I haven't find a way to do it, any idea?
----------------------------------------------------------------
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.
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 #14202: Fix(markline): markline label should have higher z
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #14202:
URL: https://github.com/apache/echarts/pull/14202#issuecomment-772575829
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/incubator-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.
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] pissang commented on a change in pull request #14202: Fix(markline): markline label should have higher z
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #14202:
URL: https://github.com/apache/echarts/pull/14202#discussion_r569505839
##########
File path: src/chart/helper/Line.ts
##########
@@ -261,7 +261,9 @@ class Line extends graphic.Group {
? lineData.getName(idx)
: isFinite(rawVal)
? round(rawVal)
- : rawVal) + ''
+ : rawVal) + '',
+ // @ts-ignore
+ z: seriesModel.getData(lineData.dataType).getItemModel(idx).get('z') + 1
Review comment:
not only `markLine` use `Line.ts`.
In fact. `z` of each label will be set with a larger value in https://github.com/apache/echarts/blob/master/src/core/echarts.ts#L2170 . We need to find out why this don't work on markLine
Also `@ts-ignore` should always be avoided unless the code is very simple and you are very sure about the input type and output type.a
##########
File path: src/chart/helper/Line.ts
##########
@@ -261,7 +261,9 @@ class Line extends graphic.Group {
? lineData.getName(idx)
: isFinite(rawVal)
? round(rawVal)
- : rawVal) + ''
+ : rawVal) + '',
+ // @ts-ignore
+ z: seriesModel.getData(lineData.dataType).getItemModel(idx).get('z') + 1
Review comment:
not only `markLine` use `Line.ts`.
In fact. `z` of each label will be set with a larger value in https://github.com/apache/echarts/blob/master/src/core/echarts.ts#L2170 . We need to find out why this does't work on markLine
Also `@ts-ignore` should always be avoided unless the code is very simple and you are very sure about the input type and output type.a
----------------------------------------------------------------
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.
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] susiwen8 closed pull request #14202: Fix(markline): markline label should have higher z
Posted by GitBox <gi...@apache.org>.
susiwen8 closed pull request #14202:
URL: https://github.com/apache/echarts/pull/14202
--
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.
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] pissang commented on a change in pull request #14202: Fix(markline): markline label should have higher z
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #14202:
URL: https://github.com/apache/echarts/pull/14202#discussion_r569510699
##########
File path: src/label/labelStyle.ts
##########
@@ -275,6 +276,7 @@ function setLabelStyle<TLabelDataIndex>(
textContent.ignore = !showNormal;
// Always create new style.
textContent.useStyle(normalStyle);
+ textContent.z = opt.z ?? textContent.z;
Review comment:
We should be careful about using `??` or `?.` because they will be transpiled to much more complex code and increase the final bundle size. In this case:
```ts
opt.z != null && (textContent.z = opt.z);
```
is clearer and won't be transpiled.
##########
File path: src/chart/helper/Line.ts
##########
@@ -261,7 +261,9 @@ class Line extends graphic.Group {
? lineData.getName(idx)
: isFinite(rawVal)
? round(rawVal)
- : rawVal) + ''
+ : rawVal) + '',
+ // @ts-ignore
+ z: seriesModel.getData(lineData.dataType).getItemModel(idx).get('z') + 1
Review comment:
not only `markLine` use `Line.ts`
Also `@ts-ignore` should always be avoided unless the code is very simple and you are very sure about the input type and output type.a
----------------------------------------------------------------
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.
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] susiwen8 commented on pull request #14202: Fix(markline): markline label should have higher z
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #14202:
URL: https://github.com/apache/echarts/pull/14202#issuecomment-773318062
@pissang It seems `markline` is a `Group` for which `z` for `textContent` has been marked as `TODO`, I haven't find a way to do it, any idea?
----------------------------------------------------------------
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.
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