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/11/17 19:07:38 UTC

[GitHub] [incubator-echarts] vially opened a new pull request #13638: fix(lines): discard extraneous data-points during lines animation

vially opened a new pull request #13638:
URL: https://github.com/apache/incubator-echarts/pull/13638


   <!-- 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. -->
   
   Discards data-points that were erroneously added to the chart during the lines animation.
   
   ### Fixed issues
   
   - #10228
   - #11633
   - #12164
   - #12433
   
   There are multiple issues currently open about this problem and they might all be related.
   
   ## Details
   
   Add a test to easily reproduce the bug where points are erroneously added to the chart during the lines animation and make a *naive* attempt at fixing it.
   
   ### Before: What was the problem?
   
   <!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
   
   This bug happens during line-animations and it makes the chart display some invalid data-points after the animation is complete. 
   
   These erroneous data-points seem to be transient and they seem to go away after some interactions with the chart (e.g.: by manually changing the data-zoom).
   
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   This is a screenshot of the bug from the added test case (there are only **two** points in the series so there should only be **one** segment):
   
   ![erroneous_line](https://user-images.githubusercontent.com/433598/99429503-67083380-2908-11eb-9499-04c97fee9b05.png)
   
   Reproduction link: https://jsfiddle.net/8kv71j0w/7/ (also see added test case in `test/lines-extraneous.html`).
   
   ### After: How is it fixed in this PR?
   
   Due to some items in the *old* series having been filtered out (e.g.: by the data-zoom component), the `rawIndex` of the **removed** items is different than the `index`. Therefore, [this condition](https://github.com/apache/incubator-echarts/blob/694ea4328fb1e131449a0c24ba54fa8139ce41c3/src/chart/line/lineAnimationDiff.ts#L141) evaluates to `true` which results in [this particular code path](https://github.com/apache/incubator-echarts/blob/694ea4328fb1e131449a0c24ba54fa8139ce41c3/src/chart/line/lineAnimationDiff.ts#L141-L155) being executed. This in turn has the effect of keeping these **removed** points in the line during (and after) the animation.
   
   <!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
   
   The naive fix as part of this pull-request was to remove that code-block all together.
   
   ***Warning***: I don't have enough context to understand the full-implications of this change and it's very likely that this is **not a good fix** for this problem.
   
   The main reason for opening this pull-request was to start a discussion and get some feedback from the developers about **what would a proper fix look like**.
   
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   
   That being said, this proposed change does seem to produce the expected result in the attached test case:
   
   ![no_erroneous_line](https://user-images.githubusercontent.com/433598/99433194-80f84500-290d-11eb-8a01-bb8b37ef0380.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
   
   See test case in: `test/lines-extraneous.html`
   
   
   ## Others
   
   ### Merging options
   
   - [ ] Please squash the commits into a single one when merge.
   
   ### Other information
   
   See the linked issues for more details about other instances where this bug happens.


----------------------------------------------------------------
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 #13638: fix(lines): discard extraneous data-points during lines animation

Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #13638:
URL: https://github.com/apache/incubator-echarts/pull/13638#issuecomment-729139251


   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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org