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 2019/12/29 14:35:06 UTC
[GitHub] [incubator-echarts] susiwen8 opened a new pull request #11942:
Clear all events when clear() was called #11938
susiwen8 opened a new pull request #11942: Clear all events when clear() was called #11938
URL: https://github.com/apache/incubator-echarts/pull/11942
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [ ] bug fixing
- [ ] new feature
- [ ] others
### What does this PR do?
Clear all existing events when clear() was called
### Fixed issues
Close #11938
## Details
### Before: What was the problem?
<img width="254" alt="Screen Shot 2019-12-29 at 22 33 15" src="https://user-images.githubusercontent.com/20318608/71558192-674ad680-2a8b-11ea-997e-974fc7093691.png">
### After: How is it fixed in this PR?
<img width="260" alt="Screen Shot 2019-12-29 at 22 32 43" src="https://user-images.githubusercontent.com/20318608/71558196-72056b80-2a8b-11ea-97d4-66d1fe60b1fa.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
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] [incubator-echarts] 100pah commented on a change in pull request
#11942: Clear all events when clear() was called #11938
Posted by GitBox <gi...@apache.org>.
100pah commented on a change in pull request #11942: Clear all events when clear() was called #11938
URL: https://github.com/apache/incubator-echarts/pull/11942#discussion_r361864501
##########
File path: src/echarts.js
##########
@@ -326,6 +326,13 @@ echartsProto._onframe = function () {
}
};
+/**
+ * @return {Object}
+ */
+echartsProto.getHandlers = function () {
+ return this._$handlers;
Review comment:
`this._$handler` is the private property of `Eventful.js`. It should only be accessible in `Eventful.js` rather than `echarts.js`.
And the method `getHandlers` should not be exposed to developers until really necessary.
----------------------------------------------------------------
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] [incubator-echarts] pissang edited a comment on issue #11942: Clear
all events when clear() was called #11938
Posted by GitBox <gi...@apache.org>.
pissang edited a comment on issue #11942: Clear all events when clear() was called #11938
URL: https://github.com/apache/incubator-echarts/pull/11942#issuecomment-569564523
Hi. Not clearing events is by design. It's not a bug.
----------------------------------------------------------------
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] [incubator-echarts] 100pah commented on a change in pull request
#11942: Clear all events when clear() was called #11938
Posted by GitBox <gi...@apache.org>.
100pah commented on a change in pull request #11942: Clear all events when clear() was called #11938
URL: https://github.com/apache/incubator-echarts/pull/11942#discussion_r361867031
##########
File path: src/echarts.js
##########
@@ -1682,6 +1689,14 @@ echartsProto.clear = function () {
disposedWarning(this.id);
return;
}
+
+ // clear all events
+ var handlers = this.getHandlers();
Review comment:
I think the handler registered by users should better not be cleared when users called `clear`.
It depends on how we define the term `clear`.
But at least, if making change, that will be a break change.
Users call unregister the listeners via calling `chart.off()`.
----------------------------------------------------------------
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] [incubator-echarts] susiwen8 commented on issue #11942: Clear all
events when clear() was called #11938
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on issue #11942: Clear all events when clear() was called #11938
URL: https://github.com/apache/incubator-echarts/pull/11942#issuecomment-569571365
@pissang @100pah Understood, 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
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] [incubator-echarts] pissang closed pull request #11942: Clear all
events when clear() was called #11938
Posted by GitBox <gi...@apache.org>.
pissang closed pull request #11942: Clear all events when clear() was called #11938
URL: https://github.com/apache/incubator-echarts/pull/11942
----------------------------------------------------------------
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] [incubator-echarts] pissang commented on issue #11942: Clear all
events when clear() was called #11938
Posted by GitBox <gi...@apache.org>.
pissang commented on issue #11942: Clear all events when clear() was called #11938
URL: https://github.com/apache/incubator-echarts/pull/11942#issuecomment-569564523
Thanks. Not clearing events is by design. It's not a bug.
----------------------------------------------------------------
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