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 2020/03/04 14:00:40 UTC
[GitHub] [incubator-echarts] FallenMax opened a new pull request #12236: fix
#9265 axis.nameGap set automatically given grid.containLabel
FallenMax opened a new pull request #12236: fix #9265 axis.nameGap set automatically given grid.containLabel
URL: https://github.com/apache/incubator-echarts/pull/12236
<!-- 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?
Fixes #9265 . Axis.nameGap will be now be calculated upon `grid.containLabel: true` and axis labels.
### Fixed issues
- #9265 xAxis.nameGap and yAxis.nameGap should be set automatically given grid.containLabel
## Details
### Before: What was the problem?
For charts with grid.containLabel set to `true`, axis name could be overlapped with axis labels, if `yAxis.nameGap` is not manually tweaked.
Here is how echarts behaves by default (quoting: https://github.com/apache/incubator-echarts/issues/9265#issuecomment-433346995):
### After: How is it fixed in this PR?
now axis' name will always placed outside `grid + axis label` rect. `nameGap` only adds some additional gap.
## Usage
### Are there any API changes?
- [ ] The API has been changed.
### Related test cases or examples to use the new APIs
NA.
## 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] plainheart closed pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
plainheart closed pull request #12236:
URL: https://github.com/apache/echarts/pull/12236
--
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] konrad-amtenbrink commented on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on pull request #12236:
URL: https://github.com/apache/echarts/pull/12236#issuecomment-1085827957
@FallenMax I am sorry to bother you again - I just had the time to look into the PR, but my collaboration invitation is expired. Could you send me a new one?
Thanks a lot!
--
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] FallenMax commented on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
FallenMax commented on pull request #12236:
URL: https://github.com/apache/echarts/pull/12236#issuecomment-1067513961
@konrad-amtenbrink No problem! If it's needed, I'v added you as collaborator to https://github.com/FallenMax/incubator-echarts, feel free to use this branch to make/push any change, or fork echarts and make a separate PR. (Sorry for not being able to finishing the PR myself)
--
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] konrad-amtenbrink commented on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on pull request #12236:
URL: https://github.com/apache/echarts/pull/12236#issuecomment-1066958449
Hey, as this is still a relevant problem - I want to implement the requested changes for this PR. Is there a way this is possible? @pissang @FallenMax
--
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] mpolverini commented on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
mpolverini commented on pull request #12236:
URL: https://github.com/apache/echarts/pull/12236#issuecomment-1056700425
This is still a relevant problem. Is there any chance that this fix is merged in the short term?
--
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] [incubator-echarts] echarts-bot[bot] commented on issue #12236: fix
#9265 axis.nameGap set automatically given grid.containLabel
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on issue #12236: fix #9265 axis.nameGap set automatically given grid.containLabel
URL: https://github.com/apache/incubator-echarts/pull/12236#issuecomment-594539644
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).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [echarts] konrad-amtenbrink edited a comment on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink edited a comment on pull request #12236:
URL: https://github.com/apache/echarts/pull/12236#issuecomment-1085951102
@FallenMax @pissang After looking at the PR I forked the repository myself and applied the changes already made. There is a new PR open for this issue. I hope this works out for everybody, if not please let me know. The new PR is: https://github.com/apache/echarts/pull/16825
So this PR is no longer needed.
--
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] konrad-amtenbrink commented on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink commented on pull request #12236:
URL: https://github.com/apache/echarts/pull/12236#issuecomment-1085951102
@FallenMax @pissang After looking at the PR I forked the repository myself and applied and adapted the changes already made. There is a new PR open for this issue. I hope this works out for everybody, if not please let me know. The new PR is: https://github.com/apache/echarts/pull/16825
So this PR is no longer needed.
--
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] konrad-amtenbrink removed a comment on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
konrad-amtenbrink removed a comment on pull request #12236:
URL: https://github.com/apache/echarts/pull/12236#issuecomment-1085827957
@FallenMax I am sorry to bother you again - I just had the time to look into the PR, but my collaboration invitation is expired. Could you send me a new one?
Thanks a lot!
--
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] [incubator-echarts] adyry commented on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
adyry commented on pull request #12236:
URL: https://github.com/apache/incubator-echarts/pull/12236#issuecomment-698909094
I was really waiting for this fix to come with `4.9.0`...
----------------------------------------------------------------
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] [incubator-echarts] FallenMax commented on a change in pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
FallenMax commented on a change in pull request #12236:
URL: https://github.com/apache/incubator-echarts/pull/12236#discussion_r469271067
##########
File path: src/component/axis/AxisBuilder.js
##########
@@ -269,7 +269,22 @@ var builders = {
var nameLocation = axisModel.get('nameLocation');
var nameDirection = opt.nameDirection;
var textStyleModel = axisModel.getModel('nameTextStyle');
- var gap = axisModel.get('nameGap') || 0;
+ // label + gap
+ var distanceToAxis = calcDistanceToAxis();
+ function calcDistanceToAxis() {
+ var axis = axisModel.axis;
+ if (axis.grid.model.get('containLabel') && !axis.model.get('axisLabel.inside')) {
+ var labelUnionRect = estimateLabelUnionRect(axis);
Review comment:
Sorry but I don't have time looking into this for now, can you make a fix if the issue is still relevant?
----------------------------------------------------------------
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] [incubator-echarts] pissang commented on a change in pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #12236:
URL: https://github.com/apache/incubator-echarts/pull/12236#discussion_r465070440
##########
File path: src/component/axis/AxisBuilder.js
##########
@@ -269,7 +269,22 @@ var builders = {
var nameLocation = axisModel.get('nameLocation');
var nameDirection = opt.nameDirection;
var textStyleModel = axisModel.getModel('nameTextStyle');
- var gap = axisModel.get('nameGap') || 0;
+ // label + gap
+ var distanceToAxis = calcDistanceToAxis();
+ function calcDistanceToAxis() {
+ var axis = axisModel.axis;
+ if (axis.grid.model.get('containLabel') && !axis.model.get('axisLabel.inside')) {
+ var labelUnionRect = estimateLabelUnionRect(axis);
Review comment:
AxisBuilder can also be used on coordinate systems other than grids. In which case, axis.grid may be null. And access grid model in the axis builder is an abstraction leak.
It's better to pass the extra gap calculated from labels from the top. Which can be a parameter in https://github.com/apache/incubator-echarts/blob/master/src/component/axis/CartesianAxisView.js#L60
----------------------------------------------------------------
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] [incubator-echarts] adyry commented on pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
adyry commented on pull request #12236:
URL: https://github.com/apache/incubator-echarts/pull/12236#issuecomment-698909094
I was really waiting for this fix to come with `4.9.0`...
----------------------------------------------------------------
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] [incubator-echarts] pissang commented on a change in pull request #12236: fix #9265 axis name overlapped with axis labels for `grid.containLabel: true`
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #12236:
URL: https://github.com/apache/incubator-echarts/pull/12236#discussion_r465070440
##########
File path: src/component/axis/AxisBuilder.js
##########
@@ -269,7 +269,22 @@ var builders = {
var nameLocation = axisModel.get('nameLocation');
var nameDirection = opt.nameDirection;
var textStyleModel = axisModel.getModel('nameTextStyle');
- var gap = axisModel.get('nameGap') || 0;
+ // label + gap
+ var distanceToAxis = calcDistanceToAxis();
+ function calcDistanceToAxis() {
+ var axis = axisModel.axis;
+ if (axis.grid.model.get('containLabel') && !axis.model.get('axisLabel.inside')) {
+ var labelUnionRect = estimateLabelUnionRect(axis);
Review comment:
AxisBuilder can also be used on coordinate systems other than grids. In which case, axis.grid may be null. And access grid model in the axis builder is an abstraction leak.
----------------------------------------------------------------
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