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/06/19 14:41:20 UTC
[GitHub] [incubator-echarts] liulinboyi opened a new pull request #12834: fix: #12812
liulinboyi opened a new pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834
<!-- 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 ONCE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
when alwaysShowContent is true change or rotation window size and restore will hide tooltip
### Fixed issues
#12812
<!--
- #xxxx: ...
-->
## Details
### Before: What was the problem?
<!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
1- display echart tooltip exemple: https://echarts.apache.org/examples/en/editor.html?c=line-tooltip-touch
2- add option 'alwaysShowContent: true' for tooltip
3- click on the chart for display tooltip
4- change window size or rotation
the tooltip move, not corresponding to the selected point position
### After: How is it fixed in this PR?
<!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
when alwaysShowContent is true change or rotation window size and restore will hide tooltip
## 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
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443369852
##########
File path: src/component/tooltip/TooltipRichContent.js
##########
@@ -21,13 +21,34 @@ import * as zrUtil from 'zrender/src/core/util';
// import Group from 'zrender/src/container/Group';
import Text from 'zrender/src/graphic/Text';
+
+function makeStyleCoord(out, zr, zrX, zrY) {
+ var zrPainter = zr && zr.painter;
+ out[0] = zrX;
+ out[1] = zrY;
+ // xy should be based on canvas root. But tooltipContent is
+ // the sibling of canvas root. So padding of ec container
+ // should be considered here.
+ var viewportRootOffset = zrPainter && zrPainter.getViewportRootOffset();
+ if (viewportRootOffset) {
+ out[0] += viewportRootOffset.offsetLeft;
+ out[1] += viewportRootOffset.offsetTop;
+ }
Review comment:
> I think there is no need for adding this logic in rich tooltip, since rich tooltip won't depend on DOM.
> This may bring a unexpected result which has a big space if add it here.
> For example:
> 
>
> If we remove this logic, it looks like normal.
> 
@plainheart All the problems are solved, Please help review, thanks!
----------------------------------------------------------------
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r444004883
##########
File path: src/component/tooltip/TooltipContent.js
##########
@@ -250,11 +252,25 @@ TooltipContent.prototype = {
if (domStyle.position !== 'absolute' && stl.position !== 'absolute') {
domStyle.position = 'relative';
}
+ var alwaysShowContent = tooltipModel.get('alwaysShowContent');
+ alwaysShowContent && this._moveTooltipIfResized();
// Hide the tooltip
// PENDING
// this.hide();
},
+ /**
+ * when alwaysShowContent is true change or rotation window size and restore will move tooltip
+ * @private
+ */
Review comment:
> Here is a suggestion for this comment
>
> ```js
> // when `alwaysShowContent` is true,
> // we should move the tooltip after chart resized
> ```
ok, i see, thank you!
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647901046
> Perhaps the `_moveTooltip` can be renamed to a more specific name like `_moveTooltipIfResized`
>
> Others all look good to me. Thanks for the wonderful work! @liulinboyi @plainheart
Ok, no problem.
----------------------------------------------------------------
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] plainheart commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443363667
##########
File path: src/component/tooltip/TooltipRichContent.js
##########
@@ -21,13 +21,34 @@ import * as zrUtil from 'zrender/src/core/util';
// import Group from 'zrender/src/container/Group';
import Text from 'zrender/src/graphic/Text';
+
+function makeStyleCoord(out, zr, zrX, zrY) {
+ var zrPainter = zr && zr.painter;
+ out[0] = zrX;
+ out[1] = zrY;
+ // xy should be based on canvas root. But tooltipContent is
+ // the sibling of canvas root. So padding of ec container
+ // should be considered here.
+ var viewportRootOffset = zrPainter && zrPainter.getViewportRootOffset();
+ if (viewportRootOffset) {
+ out[0] += viewportRootOffset.offsetLeft;
+ out[1] += viewportRootOffset.offsetTop;
+ }
Review comment:
I think there is no need for adding this logic in rich tooltip, since rich tooltip won't depend on DOM.
This may bring a unexpected result which has a big space if add it here.
For example:
If we remove this logic, it looks like normal.
----------------------------------------------------------------
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 pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647893072
Looks good to me. Thanks for the wonderful work! @liulinboyi @plainheart
----------------------------------------------------------------
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443369852
##########
File path: src/component/tooltip/TooltipRichContent.js
##########
@@ -21,13 +21,34 @@ import * as zrUtil from 'zrender/src/core/util';
// import Group from 'zrender/src/container/Group';
import Text from 'zrender/src/graphic/Text';
+
+function makeStyleCoord(out, zr, zrX, zrY) {
+ var zrPainter = zr && zr.painter;
+ out[0] = zrX;
+ out[1] = zrY;
+ // xy should be based on canvas root. But tooltipContent is
+ // the sibling of canvas root. So padding of ec container
+ // should be considered here.
+ var viewportRootOffset = zrPainter && zrPainter.getViewportRootOffset();
+ if (viewportRootOffset) {
+ out[0] += viewportRootOffset.offsetLeft;
+ out[1] += viewportRootOffset.offsetTop;
+ }
Review comment:
> I think there is no need for adding this logic in rich tooltip, since rich tooltip won't depend on DOM.
> This may bring a unexpected result which has a big space if add it here.
> For example:
> 
>
> If we remove this logic, it looks like normal.
> 
@plainheart All the problems are solved, Please help review, thanks!
----------------------------------------------------------------
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] echarts-bot[bot] commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647964985
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.
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] susiwen8 commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r442881717
##########
File path: src/echarts.js
##########
@@ -1142,8 +1142,22 @@ echartsProto.resize = function (opts) {
flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
+
+ hideTooltip.call(this);
};
+/**
+ * when alwaysShowContent is true change or rotation window size and restore will hide tooltip
+ */
+function hideTooltip() {
+ const tooltips = this._componentsViews.filter(item => item.__model.mainType === 'tooltip');
Review comment:
Please use ES3, so `const` and `filter` are not allowed to be used in this project
https://echarts.apache.org/zh/coding-standard.html#language-features
##########
File path: src/echarts.js
##########
@@ -1142,8 +1142,22 @@ echartsProto.resize = function (opts) {
flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
+
+ hideTooltip.call(this);
};
+/**
+ * when alwaysShowContent is true change or rotation window size and restore will hide tooltip
+ */
+function hideTooltip() {
+ const tooltips = this._componentsViews.filter(item => item.__model.mainType === 'tooltip');
Review comment:
Also, any property started by '_' are private, so we shouldn't directly manipulate it
----------------------------------------------------------------
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r442896796
##########
File path: src/echarts.js
##########
@@ -1142,8 +1142,22 @@ echartsProto.resize = function (opts) {
flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
+
+ hideTooltip.call(this);
};
+/**
+ * when alwaysShowContent is true change or rotation window size and restore will hide tooltip
+ */
+function hideTooltip() {
+ const tooltips = this._componentsViews.filter(item => item.__model.mainType === 'tooltip');
Review comment:
> Also, any property started by '_' are private, so we shouldn't directly manipulate it
Currently I have no other way to hide tooltip in resie
----------------------------------------------------------------
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] plainheart commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443337016
##########
File path: src/component/tooltip/TooltipContent.js
##########
@@ -250,11 +253,23 @@ TooltipContent.prototype = {
if (domStyle.position !== 'absolute' && stl.position !== 'absolute') {
domStyle.position = 'relative';
}
+ this._moveTooltip()
Review comment:
Maybe should add a judgement about whether `alwaysShowContent` is `true`
`TooltipView.js`([Line 112](https://github.com/apache/incubator-echarts/blob/master/src/component/tooltip/TooltipView.js#L112))
```js
tooltipContent.update(tooltipModel);
```
Pass the parameter `tooltipModel` to `update` function.
```js
var alwaysShowContent = tooltipModel.get('alwaysShowContent');
alwaysShowContent && this._moveTooltip();
```
----------------------------------------------------------------
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] plainheart commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647309247
Finally, the same changed should be applied to `TooltipRichContent`, which will be used when [`renderMode`](https://echarts.apache.org/en/option.html#tooltip.renderMode) is `richText`.
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647333159
> Finally, the same changes should be applied to `TooltipRichContent`, which will be used when [`renderMode`](https://echarts.apache.org/en/option.html#tooltip.renderMode) is `richText`.
@plainheart All the problems are solved, Please help review, thanks!
----------------------------------------------------------------
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] plainheart edited a comment on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart edited a comment on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647309247
Finally, the same changes should be applied to `TooltipRichContent`, which will be used when [`renderMode`](https://echarts.apache.org/en/option.html#tooltip.renderMode) is `richText`.
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647362101
> Finally, the same changes should be applied to `TooltipRichContent`, which will be used when [`renderMode`](https://echarts.apache.org/en/option.html#tooltip.renderMode) is `richText`.
@plainheart Thank you for your guidance.
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647288842
@plainheart All the problems are solved, Please help review, thanks!
----------------------------------------------------------------
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] plainheart merged pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart merged pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834
----------------------------------------------------------------
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] plainheart commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443313348
##########
File path: src/echarts.js
##########
@@ -1142,8 +1143,26 @@ echartsProto.resize = function (opts) {
flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
+
+ moveTooltip.call(this);
Review comment:
Still not sure whether it is proper to handle tooltip relocation here, maybe it's better to move these logic to `TooltipView`?
##########
File path: src/echarts.js
##########
@@ -1142,8 +1143,26 @@ echartsProto.resize = function (opts) {
flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
+
+ moveTooltip.call(this);
};
+/**
+ * when alwaysShowContent is true change or rotation window size and restore will move tooltip
+ * @private
+ */
+function moveTooltip() {
+ var tooltips = filter(this._componentsViews, function (item) {
+ return item.__model.mainType === 'tooltip';
+ });
+ each(tooltips, function (tooltip) {
+ var realLeft = tooltip._tooltipContent._styleCoord[2] * this._zr.getWidth();
+ var realRight = tooltip._tooltipContent._styleCoord[1];
+ var ratio = tooltip._tooltipContent._styleCoord[2];
+ tooltip._tooltipContent.moveTo(realLeft, realRight, ratio);
Review comment:
I can't find the usage of the parameter `ratio`, did I miss something?
----------------------------------------------------------------
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] plainheart commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647964829
> > Seems there are still two request changes from @plainheart @susiwen8 need to be resolved before this PR can ben merged
>
> @pissang All request changes are done. Can i fix other files spell error?
Merged. Thanks for your contribution!
And for the request to fix spell error, I think you are welcome and free to do it.
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647263289
> Why hide tooltip rather than relocate it to the proper coordinate?
> And I think doing it here is a bit inappropriate.
Let me have a try.
----------------------------------------------------------------
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 edited a comment on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647893072
Perhaps the `_moveTooltip` can be renamed to a more specific name like `_moveTooltipIfResized`
Others all look good to me. Thanks for the wonderful work! @liulinboyi @plainheart
----------------------------------------------------------------
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443366062
##########
File path: src/component/tooltip/TooltipRichContent.js
##########
@@ -21,13 +21,34 @@ import * as zrUtil from 'zrender/src/core/util';
// import Group from 'zrender/src/container/Group';
import Text from 'zrender/src/graphic/Text';
+
+function makeStyleCoord(out, zr, zrX, zrY) {
+ var zrPainter = zr && zr.painter;
+ out[0] = zrX;
+ out[1] = zrY;
+ // xy should be based on canvas root. But tooltipContent is
+ // the sibling of canvas root. So padding of ec container
+ // should be considered here.
+ var viewportRootOffset = zrPainter && zrPainter.getViewportRootOffset();
+ if (viewportRootOffset) {
+ out[0] += viewportRootOffset.offsetLeft;
+ out[1] += viewportRootOffset.offsetTop;
+ }
Review comment:
> I think there is no need for adding this logic in rich tooltip, since rich tooltip won't depend on DOM.
> This may bring a unexpected result which has a big space if add it here.
> For example:
> 
>
> If we remove this logic, it looks like normal.
> 
ok i will remove these code.
----------------------------------------------------------------
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] plainheart commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647394300
@liulinboyi Thank you, great work! All looks good to me now, please @pissang have a look.
----------------------------------------------------------------
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443314940
##########
File path: src/echarts.js
##########
@@ -1142,8 +1143,26 @@ echartsProto.resize = function (opts) {
flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
+
+ moveTooltip.call(this);
Review comment:
> Still not sure whether it is proper to handle tooltip relocation here, maybe it's better to move these logic to `TooltipView`?
Let me have a try.
----------------------------------------------------------------
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] plainheart commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443994879
##########
File path: test/tooltip-windowResize.html
##########
@@ -0,0 +1,383 @@
+<!DOCTYPE html>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+
+<html>
+
+<head>
+ <meta charset="utf-8">
+ <meta name="viewport" content="width=device-width, initial-scale=1" />
+ <script src="lib/esl.js"></script>
+ <script src="lib/config.js"></script>
+ <script src="lib/facePrint.js"></script>
+ <script src="lib/testHelper.js"></script>
+ <script src="tooltipTestHelper.js"></script>
+ <link rel="stylesheet" href="lib/reset.css" />
+</head>
+
+<body>
+ <style>
+ .test-title {
+ background: #fff;
+ }
+
+ .test-chart {
+ margin: 0 auto;
+ }
+
+ </style>
+
+
+ <div id="main0"></div>
+
+ <div id="main1"></div>
+
+
+ <script>
+ require([
+ 'echarts'
+ // 'echarts/chart/bar',
+ // 'echarts/component/polar',
+ // 'zrender/vml/vml'
+ ], function (echarts) {
+ var base = +new Date(2016, 9, 3);
+ var oneDay = 24 * 3600 * 1000;
+ var valueBase = Math.random() * 300;
+ var valueBase2 = Math.random() * 50;
+ var data = [];
+ var data2 = [];
+
+ for (var i = 1; i < 10; i++) {
+ var now = new Date(base += oneDay);
+ var dayStr = [now.getFullYear(), now.getMonth() + 1, now.getDate()].join('-');
+
+ valueBase = Math.round((Math.random() - 0.5) * 20 + valueBase);
+ valueBase <= 0 && (valueBase = Math.random() * 300);
+ data.push([dayStr, valueBase]);
+
+ valueBase2 = Math.round((Math.random() - 0.5) * 20 + valueBase2);
+ valueBase2 <= 0 && (valueBase2 = Math.random() * 50);
+ data2.push([dayStr, valueBase2]);
+ }
+ option = {
+ animation: false,
+ title: {
+ left: 'center',
+ text: '触屏 tooltip 和 dataZoom 示例',
+ subtext: '"tootip" and "dataZoom" on mobile device',
+ },
+ legend: {
+ top: 'bottom',
+ data: ['意向']
+ },
+ tooltip: {
+ alwaysShowContent: true,
+ triggerOn: 'none',
+ position: function (pt) {
+ return [pt[0], 130];
+ }
+ },
+ toolbox: {
+ left: 'center',
+ itemSize: 25,
+ top: 55,
+ feature: {
+ dataZoom: {
+ yAxisIndex: 'none'
+ },
+ restore: {}
+ }
+ },
+ xAxis: {
+ type: 'time',
+ // boundaryGap: [0, 0],
+ axisPointer: {
+ value: '2016-10-7',
+ snap: true,
+ lineStyle: {
+ color: '#004E52',
+ opacity: 0.5,
+ width: 2
+ },
+ label: {
+ show: true,
+ formatter: function (params) {
+ return echarts.format.formatTime('yyyy-MM-dd', params.value);
+ },
+ backgroundColor: '#004E52'
+ },
+ handle: {
+ show: true,
+ color: '#004E52'
+ }
+ },
+ splitLine: {
+ show: false
+ }
+ },
+ yAxis: {
+ type: 'value',
+ axisTick: {
+ inside: true
+ },
+ splitLine: {
+ show: false
+ },
+ axisLabel: {
+ inside: true,
+ formatter: '{value}\n'
+ },
+ z: 10
+ },
+ grid: {
+ top: 110,
+ left: 15,
+ right: 15,
+ height: 160
+ },
+ dataZoom: [{
+ type: 'inside',
+ throttle: 50
+ }],
+ series: [{
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#8ec6ad'
+ },
+ stack: 'a',
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#8ec6ad'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data
+ },
+ {
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ stack: 'a',
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#d68262'
+ },
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#d68262'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data2
+ }
+
+ ]
+ };
+
+ var chart = testHelper.create(echarts, 'main0', {
+ option: option
+ });
+ chart.setOption(option, true);
+ window.onresize = function () {
+ chart.resize();
+ }
+ });
+
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ // 'echarts/chart/bar',
+ // 'echarts/component/polar',
+ // 'zrender/vml/vml'
+ ], function (echarts) {
+ var base = +new Date(2016, 9, 3);
+ var oneDay = 24 * 3600 * 1000;
+ var valueBase = Math.random() * 300;
+ var valueBase2 = Math.random() * 50;
+ var data = [];
+ var data2 = [];
+
+ for (var i = 1; i < 10; i++) {
+ var now = new Date(base += oneDay);
+ var dayStr = [now.getFullYear(), now.getMonth() + 1, now.getDate()].join('-');
+
+ valueBase = Math.round((Math.random() - 0.5) * 20 + valueBase);
+ valueBase <= 0 && (valueBase = Math.random() * 300);
+ data.push([dayStr, valueBase]);
+
+ valueBase2 = Math.round((Math.random() - 0.5) * 20 + valueBase2);
+ valueBase2 <= 0 && (valueBase2 = Math.random() * 50);
+ data2.push([dayStr, valueBase2]);
+ }
+ option = {
+ animation: false,
+ title: {
+ left: 'center',
+ text: '触屏 tooltip 和 dataZoom 示例',
+ subtext: '"tootip" and "dataZoom" on mobile device',
+ },
+ legend: {
+ top: 'bottom',
+ data: ['意向']
+ },
+ tooltip: {
+ alwaysShowContent: true,
+ renderMode: 'richText',
+ triggerOn: 'none',
+ position: function (pt) {
+ return [pt[0], 130];
+ }
+ },
+ toolbox: {
+ left: 'center',
+ itemSize: 25,
+ top: 55,
+ feature: {
+ dataZoom: {
+ yAxisIndex: 'none'
+ },
+ restore: {}
+ }
+ },
+ xAxis: {
+ type: 'time',
+ // boundaryGap: [0, 0],
+ axisPointer: {
+ value: '2016-10-7',
+ snap: true,
+ lineStyle: {
+ color: '#004E52',
+ opacity: 0.5,
+ width: 2
+ },
+ label: {
+ show: true,
+ formatter: function (params) {
+ return echarts.format.formatTime('yyyy-MM-dd', params.value);
+ },
+ backgroundColor: '#004E52'
+ },
+ handle: {
+ show: true,
+ color: '#004E52'
+ }
+ },
+ splitLine: {
+ show: false
+ }
+ },
+ yAxis: {
+ type: 'value',
+ axisTick: {
+ inside: true
+ },
+ splitLine: {
+ show: false
+ },
+ axisLabel: {
+ inside: true,
+ formatter: '{value}\n'
+ },
+ z: 10
+ },
+ grid: {
+ top: 110,
+ left: 15,
+ right: 15,
+ height: 160
+ },
+ dataZoom: [{
+ type: 'inside',
+ throttle: 50
+ }],
+ series: [{
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#8ec6ad'
+ },
+ stack: 'a',
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#8ec6ad'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data
+ },
+ {
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ stack: 'a',
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#d68262'
+ },
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#d68262'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data2
+ }
+
+ ]
+ };
+
+ var chart = testHelper.create(echarts, 'main1', {
+ option: option
+ });
+ chart.setOption(option, true);
+ window.onresize = function () {
+ chart.resize();
+ }
Review comment:
Consider using `window.addEventListener`, `window` is shared with these two test cases, `window.onresize` in the bottom test case will override the top one.
----------------------------------------------------------------
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 pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647923354
Seems there are still two request changes from @plainheart @susiwen8 need to be resolved before this PR can ben merged
----------------------------------------------------------------
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443349449
##########
File path: src/component/tooltip/TooltipContent.js
##########
@@ -250,11 +253,23 @@ TooltipContent.prototype = {
if (domStyle.position !== 'absolute' && stl.position !== 'absolute') {
domStyle.position = 'relative';
}
+ this._moveTooltip()
Review comment:
> Maybe should add a judgement about whether `alwaysShowContent` is `true`
> `TooltipView.js`([Line 112](https://github.com/apache/incubator-echarts/blob/master/src/component/tooltip/TooltipView.js#L112))
>
> ```js
> tooltipContent.update(tooltipModel);
> ```
>
> Pass the parameter `tooltipModel` to `update` function.
>
> ```js
> var alwaysShowContent = tooltipModel.get('alwaysShowContent');
> alwaysShowContent && this._moveTooltip();
> ```
ok i see. thank you!
----------------------------------------------------------------
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] plainheart edited a comment on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart edited a comment on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647964829
> > Seems there are still two request changes from @plainheart @susiwen8 need to be resolved before this PR can ben merged
>
> @pissang All request changes are done. Can i fix other files spell error?
Merged. Thanks for your contribution!
And for the request to fix spelling error, I think you are welcome and free to do it.
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647399422
> @liulinboyi Would you mind providing a test case for this?
Let me have a try.
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647934695
> Seems there are still two request changes from @plainheart @susiwen8 need to be resolved before this PR can ben merged
@pissang All request changes are done. Can i fix other files spell error?
----------------------------------------------------------------
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] plainheart commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647395874
@liulinboyi Would you mind providing a test case for 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.
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r442896578
##########
File path: src/echarts.js
##########
@@ -1142,8 +1142,22 @@ echartsProto.resize = function (opts) {
flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
+
+ hideTooltip.call(this);
};
+/**
+ * when alwaysShowContent is true change or rotation window size and restore will hide tooltip
+ */
+function hideTooltip() {
+ const tooltips = this._componentsViews.filter(item => item.__model.mainType === 'tooltip');
Review comment:
Currently I have no other way to hide tooltip in resie.
----------------------------------------------------------------
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] plainheart commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647208160
Why hide tooltip rather than relocate it to the proper coordinate?
And I think doing it here is a bit inappropriate.
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647495244
> @liulinboyi Would you mind providing a test case for this?
@plainheart Test case is done, Please help review, thanks!
----------------------------------------------------------------
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443314436
##########
File path: src/echarts.js
##########
@@ -1142,8 +1143,26 @@ echartsProto.resize = function (opts) {
flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
+
+ moveTooltip.call(this);
};
+/**
+ * when alwaysShowContent is true change or rotation window size and restore will move tooltip
+ * @private
+ */
+function moveTooltip() {
+ var tooltips = filter(this._componentsViews, function (item) {
+ return item.__model.mainType === 'tooltip';
+ });
+ each(tooltips, function (tooltip) {
+ var realLeft = tooltip._tooltipContent._styleCoord[2] * this._zr.getWidth();
+ var realRight = tooltip._tooltipContent._styleCoord[1];
+ var ratio = tooltip._tooltipContent._styleCoord[2];
+ tooltip._tooltipContent.moveTo(realLeft, realRight, ratio);
Review comment:
> I can't find the usage of the parameter `ratio`, did I miss something?
The code has just been updated to remove this parameter.
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647268371
> Why hide tooltip rather than relocate it to the proper coordinate?
> And I think doing it here is a bit inappropriate.
@plainheart The effect is as follows，Please help review, thanks!
----------------------------------------------------------------
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] liulinboyi commented on pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#issuecomment-647916698
> Perhaps the `_moveTooltip` can be renamed to a more specific name like `_moveTooltipIfResized`
>
> Others all look good to me. Thanks for the wonderful work! @liulinboyi @plainheart
Thank you for your guidance. @pissang I have fix some spell error and the _moveTooltip was renamed, Please help review, thanks!
----------------------------------------------------------------
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] plainheart commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443994879
##########
File path: test/tooltip-windowResize.html
##########
@@ -0,0 +1,383 @@
+<!DOCTYPE html>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+
+<html>
+
+<head>
+ <meta charset="utf-8">
+ <meta name="viewport" content="width=device-width, initial-scale=1" />
+ <script src="lib/esl.js"></script>
+ <script src="lib/config.js"></script>
+ <script src="lib/facePrint.js"></script>
+ <script src="lib/testHelper.js"></script>
+ <script src="tooltipTestHelper.js"></script>
+ <link rel="stylesheet" href="lib/reset.css" />
+</head>
+
+<body>
+ <style>
+ .test-title {
+ background: #fff;
+ }
+
+ .test-chart {
+ margin: 0 auto;
+ }
+
+ </style>
+
+
+ <div id="main0"></div>
+
+ <div id="main1"></div>
+
+
+ <script>
+ require([
+ 'echarts'
+ // 'echarts/chart/bar',
+ // 'echarts/component/polar',
+ // 'zrender/vml/vml'
+ ], function (echarts) {
+ var base = +new Date(2016, 9, 3);
+ var oneDay = 24 * 3600 * 1000;
+ var valueBase = Math.random() * 300;
+ var valueBase2 = Math.random() * 50;
+ var data = [];
+ var data2 = [];
+
+ for (var i = 1; i < 10; i++) {
+ var now = new Date(base += oneDay);
+ var dayStr = [now.getFullYear(), now.getMonth() + 1, now.getDate()].join('-');
+
+ valueBase = Math.round((Math.random() - 0.5) * 20 + valueBase);
+ valueBase <= 0 && (valueBase = Math.random() * 300);
+ data.push([dayStr, valueBase]);
+
+ valueBase2 = Math.round((Math.random() - 0.5) * 20 + valueBase2);
+ valueBase2 <= 0 && (valueBase2 = Math.random() * 50);
+ data2.push([dayStr, valueBase2]);
+ }
+ option = {
+ animation: false,
+ title: {
+ left: 'center',
+ text: '触屏 tooltip 和 dataZoom 示例',
+ subtext: '"tootip" and "dataZoom" on mobile device',
+ },
+ legend: {
+ top: 'bottom',
+ data: ['意向']
+ },
+ tooltip: {
+ alwaysShowContent: true,
+ triggerOn: 'none',
+ position: function (pt) {
+ return [pt[0], 130];
+ }
+ },
+ toolbox: {
+ left: 'center',
+ itemSize: 25,
+ top: 55,
+ feature: {
+ dataZoom: {
+ yAxisIndex: 'none'
+ },
+ restore: {}
+ }
+ },
+ xAxis: {
+ type: 'time',
+ // boundaryGap: [0, 0],
+ axisPointer: {
+ value: '2016-10-7',
+ snap: true,
+ lineStyle: {
+ color: '#004E52',
+ opacity: 0.5,
+ width: 2
+ },
+ label: {
+ show: true,
+ formatter: function (params) {
+ return echarts.format.formatTime('yyyy-MM-dd', params.value);
+ },
+ backgroundColor: '#004E52'
+ },
+ handle: {
+ show: true,
+ color: '#004E52'
+ }
+ },
+ splitLine: {
+ show: false
+ }
+ },
+ yAxis: {
+ type: 'value',
+ axisTick: {
+ inside: true
+ },
+ splitLine: {
+ show: false
+ },
+ axisLabel: {
+ inside: true,
+ formatter: '{value}\n'
+ },
+ z: 10
+ },
+ grid: {
+ top: 110,
+ left: 15,
+ right: 15,
+ height: 160
+ },
+ dataZoom: [{
+ type: 'inside',
+ throttle: 50
+ }],
+ series: [{
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#8ec6ad'
+ },
+ stack: 'a',
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#8ec6ad'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data
+ },
+ {
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ stack: 'a',
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#d68262'
+ },
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#d68262'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data2
+ }
+
+ ]
+ };
+
+ var chart = testHelper.create(echarts, 'main0', {
+ option: option
+ });
+ chart.setOption(option, true);
+ window.onresize = function () {
+ chart.resize();
+ }
+ });
+
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ // 'echarts/chart/bar',
+ // 'echarts/component/polar',
+ // 'zrender/vml/vml'
+ ], function (echarts) {
+ var base = +new Date(2016, 9, 3);
+ var oneDay = 24 * 3600 * 1000;
+ var valueBase = Math.random() * 300;
+ var valueBase2 = Math.random() * 50;
+ var data = [];
+ var data2 = [];
+
+ for (var i = 1; i < 10; i++) {
+ var now = new Date(base += oneDay);
+ var dayStr = [now.getFullYear(), now.getMonth() + 1, now.getDate()].join('-');
+
+ valueBase = Math.round((Math.random() - 0.5) * 20 + valueBase);
+ valueBase <= 0 && (valueBase = Math.random() * 300);
+ data.push([dayStr, valueBase]);
+
+ valueBase2 = Math.round((Math.random() - 0.5) * 20 + valueBase2);
+ valueBase2 <= 0 && (valueBase2 = Math.random() * 50);
+ data2.push([dayStr, valueBase2]);
+ }
+ option = {
+ animation: false,
+ title: {
+ left: 'center',
+ text: '触屏 tooltip 和 dataZoom 示例',
+ subtext: '"tootip" and "dataZoom" on mobile device',
+ },
+ legend: {
+ top: 'bottom',
+ data: ['意向']
+ },
+ tooltip: {
+ alwaysShowContent: true,
+ renderMode: 'richText',
+ triggerOn: 'none',
+ position: function (pt) {
+ return [pt[0], 130];
+ }
+ },
+ toolbox: {
+ left: 'center',
+ itemSize: 25,
+ top: 55,
+ feature: {
+ dataZoom: {
+ yAxisIndex: 'none'
+ },
+ restore: {}
+ }
+ },
+ xAxis: {
+ type: 'time',
+ // boundaryGap: [0, 0],
+ axisPointer: {
+ value: '2016-10-7',
+ snap: true,
+ lineStyle: {
+ color: '#004E52',
+ opacity: 0.5,
+ width: 2
+ },
+ label: {
+ show: true,
+ formatter: function (params) {
+ return echarts.format.formatTime('yyyy-MM-dd', params.value);
+ },
+ backgroundColor: '#004E52'
+ },
+ handle: {
+ show: true,
+ color: '#004E52'
+ }
+ },
+ splitLine: {
+ show: false
+ }
+ },
+ yAxis: {
+ type: 'value',
+ axisTick: {
+ inside: true
+ },
+ splitLine: {
+ show: false
+ },
+ axisLabel: {
+ inside: true,
+ formatter: '{value}\n'
+ },
+ z: 10
+ },
+ grid: {
+ top: 110,
+ left: 15,
+ right: 15,
+ height: 160
+ },
+ dataZoom: [{
+ type: 'inside',
+ throttle: 50
+ }],
+ series: [{
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#8ec6ad'
+ },
+ stack: 'a',
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#8ec6ad'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data
+ },
+ {
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ stack: 'a',
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#d68262'
+ },
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#d68262'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data2
+ }
+
+ ]
+ };
+
+ var chart = testHelper.create(echarts, 'main1', {
+ option: option
+ });
+ chart.setOption(option, true);
+ window.onresize = function () {
+ chart.resize();
+ }
Review comment:
Consider using `window.addEventListener`, `window` is shard with these two test cases, `window.onresize` in the bottom test case will override the top one.
----------------------------------------------------------------
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] plainheart commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443997937
##########
File path: src/component/tooltip/TooltipContent.js
##########
@@ -250,11 +252,25 @@ TooltipContent.prototype = {
if (domStyle.position !== 'absolute' && stl.position !== 'absolute') {
domStyle.position = 'relative';
}
+ var alwaysShowContent = tooltipModel.get('alwaysShowContent');
+ alwaysShowContent && this._moveTooltipIfResized();
// Hide the tooltip
// PENDING
// this.hide();
},
+ /**
+ * when alwaysShowContent is true change or rotation window size and restore will move tooltip
+ * @private
+ */
Review comment:
Here is a suggestion for this comment
```js
// when `alwaysShowContent` is true,
// we should move the tooltip after chart resized
```
----------------------------------------------------------------
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] liulinboyi commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
liulinboyi commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443999449
##########
File path: test/tooltip-windowResize.html
##########
@@ -0,0 +1,383 @@
+<!DOCTYPE html>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+
+<html>
+
+<head>
+ <meta charset="utf-8">
+ <meta name="viewport" content="width=device-width, initial-scale=1" />
+ <script src="lib/esl.js"></script>
+ <script src="lib/config.js"></script>
+ <script src="lib/facePrint.js"></script>
+ <script src="lib/testHelper.js"></script>
+ <script src="tooltipTestHelper.js"></script>
+ <link rel="stylesheet" href="lib/reset.css" />
+</head>
+
+<body>
+ <style>
+ .test-title {
+ background: #fff;
+ }
+
+ .test-chart {
+ margin: 0 auto;
+ }
+
+ </style>
+
+
+ <div id="main0"></div>
+
+ <div id="main1"></div>
+
+
+ <script>
+ require([
+ 'echarts'
+ // 'echarts/chart/bar',
+ // 'echarts/component/polar',
+ // 'zrender/vml/vml'
+ ], function (echarts) {
+ var base = +new Date(2016, 9, 3);
+ var oneDay = 24 * 3600 * 1000;
+ var valueBase = Math.random() * 300;
+ var valueBase2 = Math.random() * 50;
+ var data = [];
+ var data2 = [];
+
+ for (var i = 1; i < 10; i++) {
+ var now = new Date(base += oneDay);
+ var dayStr = [now.getFullYear(), now.getMonth() + 1, now.getDate()].join('-');
+
+ valueBase = Math.round((Math.random() - 0.5) * 20 + valueBase);
+ valueBase <= 0 && (valueBase = Math.random() * 300);
+ data.push([dayStr, valueBase]);
+
+ valueBase2 = Math.round((Math.random() - 0.5) * 20 + valueBase2);
+ valueBase2 <= 0 && (valueBase2 = Math.random() * 50);
+ data2.push([dayStr, valueBase2]);
+ }
+ option = {
+ animation: false,
+ title: {
+ left: 'center',
+ text: '触屏 tooltip 和 dataZoom 示例',
+ subtext: '"tootip" and "dataZoom" on mobile device',
+ },
+ legend: {
+ top: 'bottom',
+ data: ['意向']
+ },
+ tooltip: {
+ alwaysShowContent: true,
+ triggerOn: 'none',
+ position: function (pt) {
+ return [pt[0], 130];
+ }
+ },
+ toolbox: {
+ left: 'center',
+ itemSize: 25,
+ top: 55,
+ feature: {
+ dataZoom: {
+ yAxisIndex: 'none'
+ },
+ restore: {}
+ }
+ },
+ xAxis: {
+ type: 'time',
+ // boundaryGap: [0, 0],
+ axisPointer: {
+ value: '2016-10-7',
+ snap: true,
+ lineStyle: {
+ color: '#004E52',
+ opacity: 0.5,
+ width: 2
+ },
+ label: {
+ show: true,
+ formatter: function (params) {
+ return echarts.format.formatTime('yyyy-MM-dd', params.value);
+ },
+ backgroundColor: '#004E52'
+ },
+ handle: {
+ show: true,
+ color: '#004E52'
+ }
+ },
+ splitLine: {
+ show: false
+ }
+ },
+ yAxis: {
+ type: 'value',
+ axisTick: {
+ inside: true
+ },
+ splitLine: {
+ show: false
+ },
+ axisLabel: {
+ inside: true,
+ formatter: '{value}\n'
+ },
+ z: 10
+ },
+ grid: {
+ top: 110,
+ left: 15,
+ right: 15,
+ height: 160
+ },
+ dataZoom: [{
+ type: 'inside',
+ throttle: 50
+ }],
+ series: [{
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#8ec6ad'
+ },
+ stack: 'a',
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#8ec6ad'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data
+ },
+ {
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ stack: 'a',
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#d68262'
+ },
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#d68262'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data2
+ }
+
+ ]
+ };
+
+ var chart = testHelper.create(echarts, 'main0', {
+ option: option
+ });
+ chart.setOption(option, true);
+ window.onresize = function () {
+ chart.resize();
+ }
+ });
+
+ </script>
+
+ <script>
+ require([
+ 'echarts'
+ // 'echarts/chart/bar',
+ // 'echarts/component/polar',
+ // 'zrender/vml/vml'
+ ], function (echarts) {
+ var base = +new Date(2016, 9, 3);
+ var oneDay = 24 * 3600 * 1000;
+ var valueBase = Math.random() * 300;
+ var valueBase2 = Math.random() * 50;
+ var data = [];
+ var data2 = [];
+
+ for (var i = 1; i < 10; i++) {
+ var now = new Date(base += oneDay);
+ var dayStr = [now.getFullYear(), now.getMonth() + 1, now.getDate()].join('-');
+
+ valueBase = Math.round((Math.random() - 0.5) * 20 + valueBase);
+ valueBase <= 0 && (valueBase = Math.random() * 300);
+ data.push([dayStr, valueBase]);
+
+ valueBase2 = Math.round((Math.random() - 0.5) * 20 + valueBase2);
+ valueBase2 <= 0 && (valueBase2 = Math.random() * 50);
+ data2.push([dayStr, valueBase2]);
+ }
+ option = {
+ animation: false,
+ title: {
+ left: 'center',
+ text: '触屏 tooltip 和 dataZoom 示例',
+ subtext: '"tootip" and "dataZoom" on mobile device',
+ },
+ legend: {
+ top: 'bottom',
+ data: ['意向']
+ },
+ tooltip: {
+ alwaysShowContent: true,
+ renderMode: 'richText',
+ triggerOn: 'none',
+ position: function (pt) {
+ return [pt[0], 130];
+ }
+ },
+ toolbox: {
+ left: 'center',
+ itemSize: 25,
+ top: 55,
+ feature: {
+ dataZoom: {
+ yAxisIndex: 'none'
+ },
+ restore: {}
+ }
+ },
+ xAxis: {
+ type: 'time',
+ // boundaryGap: [0, 0],
+ axisPointer: {
+ value: '2016-10-7',
+ snap: true,
+ lineStyle: {
+ color: '#004E52',
+ opacity: 0.5,
+ width: 2
+ },
+ label: {
+ show: true,
+ formatter: function (params) {
+ return echarts.format.formatTime('yyyy-MM-dd', params.value);
+ },
+ backgroundColor: '#004E52'
+ },
+ handle: {
+ show: true,
+ color: '#004E52'
+ }
+ },
+ splitLine: {
+ show: false
+ }
+ },
+ yAxis: {
+ type: 'value',
+ axisTick: {
+ inside: true
+ },
+ splitLine: {
+ show: false
+ },
+ axisLabel: {
+ inside: true,
+ formatter: '{value}\n'
+ },
+ z: 10
+ },
+ grid: {
+ top: 110,
+ left: 15,
+ right: 15,
+ height: 160
+ },
+ dataZoom: [{
+ type: 'inside',
+ throttle: 50
+ }],
+ series: [{
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#8ec6ad'
+ },
+ stack: 'a',
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#8ec6ad'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data
+ },
+ {
+ name: '模拟数据',
+ type: 'line',
+ smooth: true,
+ stack: 'a',
+ symbol: 'circle',
+ symbolSize: 5,
+ sampling: 'average',
+ itemStyle: {
+ color: '#d68262'
+ },
+ areaStyle: {
+ color: new echarts.graphic.LinearGradient(0, 0, 0, 1, [{
+ offset: 0,
+ color: '#d68262'
+ }, {
+ offset: 1,
+ color: '#ffe'
+ }])
+ },
+ data: data2
+ }
+
+ ]
+ };
+
+ var chart = testHelper.create(echarts, 'main1', {
+ option: option
+ });
+ chart.setOption(option, true);
+ window.onresize = function () {
+ chart.resize();
+ }
Review comment:
> Consider using `window.addEventListener`, `window` is shard with these two test cases, `window.onresize` in the bottom test case will override the top one.
ok, i see, thank you!
----------------------------------------------------------------
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] plainheart commented on a change in pull request #12834: fix: #12812
Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12834:
URL: https://github.com/apache/incubator-echarts/pull/12834#discussion_r443329746
##########
File path: src/component/tooltip/TooltipContent.js
##########
@@ -250,11 +253,23 @@ TooltipContent.prototype = {
if (domStyle.position !== 'absolute' && stl.position !== 'absolute') {
domStyle.position = 'relative';
}
+ this._moveTooltip()
// Hide the tooltip
// PENDING
// this.hide();
},
+ /**
+ * when alwaysShowContent is true change or rotation window size and restore will move tooltip
+ * @private
+ */
+ _moveTooltip: function() {
+ var ratio = this._styleCoord[2]; // The ratio of left to width
+ var realLeft = ratio * this._zr.getWidth();
+ var realRight = this._styleCoord[1];
+ this.moveTo(realLeft, realRight);
+ },
Review comment:
The height ratio also should be handled.
```suggestion
_moveTooltip: function() {
var ratioX = this._styleCoord[2];
var ratioY = this._styleCoord[3];
var realX = ratioX * this._zr.getWidth();
var realY = ratioY * this._zr.getHeight();
this.moveTo(realX, realY);
},
```
##########
File path: src/component/tooltip/TooltipContent.js
##########
@@ -131,18 +131,21 @@ function makeStyleCoord(out, zr, appendToBody, zrX, zrY) {
if (zrViewportRoot) {
// Some APPs might use scale on body, so we support CSS transform here.
domUtil.transformLocalCoord(out, zrViewportRoot, document.body, zrX, zrY);
+ out[2] = zrX / zr.getWidth(); // The ratio of left to width
}
}
else {
out[0] = zrX;
out[1] = zrY;
+ out[2] = zrX / zr.getWidth(); // The ratio of left to width
// xy should be based on canvas root. But tooltipContent is
// the sibling of canvas root. So padding of ec container
// should be considered here.
var viewportRootOffset = zrPainter && zrPainter.getViewportRootOffset();
if (viewportRootOffset) {
out[0] += viewportRootOffset.offsetLeft;
out[1] += viewportRootOffset.offsetTop;
+ out[2] = out[0] / zr.getWidth(); // The ratio of left to width
}
}
Review comment:
We can move these logic to the very bottom of `makeStyleCoord` .
```js
out[2] = out[0] / zr.getWidth();
out[3] = out[1] / zr.getHeight();
```
##########
File path: src/component/tooltip/TooltipContent.js
##########
@@ -169,7 +172,7 @@ function TooltipContent(container, api, opt) {
var zr = this._zr = api.getZr();
var appendToBody = this._appendToBody = opt && opt.appendToBody;
- this._styleCoord = [0, 0];
+ this._styleCoord = [0, 0, 0];
Review comment:
Looks good. But it still needs a ratio of `height` to handle the height change and relocate the tooltip.
So `_styleCoord` should be `[0, 0, 0, 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