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/04/02 03:09:39 UTC

[GitHub] [incubator-echarts] plainheart opened a new pull request #12367: fix(tree): the symbols of image type do not display at the first rendering. close apache#12279.

plainheart opened a new pull request #12367: fix(tree): the symbols of image type do not display at the first rendering. close apache#12279.
URL: https://github.com/apache/incubator-echarts/pull/12367
 
 
   <!-- 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?
   
   Fix the issue #12279.
   
   
   ### Fixed issues
   
   - #12279
   
   
   ## Details
   
   ### Before: What was the problem?
   The symbols of image type do not display at the first rendering. Please refer to #12279 for more details.
   
   
   ![Incorrect View](https://user-images.githubusercontent.com/26999792/78205580-11912a80-74cf-11ea-83a7-c40fa4186283.jpg)
   
   
   ### After: How is it fixed in this PR?
   
   ![Correct View](https://user-images.githubusercontent.com/26999792/78206752-45218400-74d2-11ea-87fe-abf11b5eff53.png)
   
   After creating the instance of `SymbolClz`, update its data immediately but not wait until the next update rendering if the type of symbol is image.
   
   ![image](https://user-images.githubusercontent.com/26999792/78206688-1b685d00-74d2-11ea-9617-3d3d014a3f51.png)
   
   
   ### Related test cases
   
   Refer to `test/tree-image2.html` for a test case.
   

----------------------------------------------------------------
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] echarts-bot[bot] commented on issue #12367: fix(tree): the symbols of image type do not display at the first rendering. close apache#12279.

Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on issue #12367: fix(tree): the symbols of image type do not display at the first rendering. close apache#12279.
URL: https://github.com/apache/incubator-echarts/pull/12367#issuecomment-607596181
 
 
   Thanks for your contribution!
   The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/incubator-echarts/wiki/How-to-make-a-pull-request).
   
   The pull request is marked to be `PR: author is committer` because you are a committer of this project.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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 pull request #12367: fix(tree): the symbols of image type do not display at the first rendering. close #12279.

Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #12367:
URL: https://github.com/apache/incubator-echarts/pull/12367#issuecomment-647268598


   Looks good


----------------------------------------------------------------
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 merged pull request #12367: fix(tree): the symbols of image type do not display at the first rendering. close #12279.

Posted by GitBox <gi...@apache.org>.
pissang merged pull request #12367:
URL: https://github.com/apache/incubator-echarts/pull/12367


   


----------------------------------------------------------------
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] 100pah commented on a change in pull request #12367: fix(tree): the symbols of image type do not display at the first rendering. close #12279.

Posted by GitBox <gi...@apache.org>.
100pah commented on a change in pull request #12367:
URL: https://github.com/apache/incubator-echarts/pull/12367#discussion_r432942719



##########
File path: src/chart/tree/TreeView.js
##########
@@ -379,9 +379,8 @@ function updateNode(data, dataIndex, symbolEl, group, seriesModel, seriesScope)
         // Fix #12279.
         // if the type of symbol is image,
         // update symbol's data immediately but not wait until the next update rendering.
-        var symbolType = symbolEl._symbolType;
-        symbolType.indexOf('image://') === 0 
-            && symbolEl.updateData(data, dataIndex, seriesScope);
+        var symbolType = symbolEl.getSymbolPath().type;
+        symbolType === 'image' && symbolEl.updateData(data, dataIndex, seriesScope);

Review comment:
       @plainheart 
   `updateData` has been called in the constructor of `Symbol`, I think it should better not be called again here.
   The actual reason of this issue seams to be in: 
   https://github.com/apache/incubator-echarts/blob/4.8.0/src/chart/helper/Symbol.js#L241
   The reset of "image" symbol should not set `opacity` to `null`.
   `null` is an invalid value for `opacity`.
   I think here the opacity should be reset as `1`. 
   And that can fix this issue.
   




----------------------------------------------------------------
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 #12367: fix(tree): the symbols of image type do not display at the first rendering. close #12279.

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12367:
URL: https://github.com/apache/incubator-echarts/pull/12367#discussion_r432943891



##########
File path: src/chart/tree/TreeView.js
##########
@@ -379,9 +379,8 @@ function updateNode(data, dataIndex, symbolEl, group, seriesModel, seriesScope)
         // Fix #12279.
         // if the type of symbol is image,
         // update symbol's data immediately but not wait until the next update rendering.
-        var symbolType = symbolEl._symbolType;
-        symbolType.indexOf('image://') === 0 
-            && symbolEl.updateData(data, dataIndex, seriesScope);
+        var symbolType = symbolEl.getSymbolPath().type;
+        symbolType === 'image' && symbolEl.updateData(data, dataIndex, seriesScope);

Review comment:
       Yes, you are totally right. I've made a new commit for this. https://github.com/apache/incubator-echarts/pull/12367/commits/48d54d8b604418b97b74e4da688c6a9699084d61




----------------------------------------------------------------
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 #12367: fix(tree): the symbols of image type do not display at the first rendering. close #12279.

Posted by GitBox <gi...@apache.org>.
plainheart commented on a change in pull request #12367:
URL: https://github.com/apache/incubator-echarts/pull/12367#discussion_r425930387



##########
File path: src/chart/tree/TreeView.js
##########
@@ -379,9 +379,8 @@ function updateNode(data, dataIndex, symbolEl, group, seriesModel, seriesScope)
         // Fix #12279.
         // if the type of symbol is image,
         // update symbol's data immediately but not wait until the next update rendering.
-        var symbolType = symbolEl._symbolType;
-        symbolType.indexOf('image://') === 0 
-            && symbolEl.updateData(data, dataIndex, seriesScope);
+        var symbolType = symbolEl.getSymbolPath().type;
+        symbolType === 'image' && symbolEl.updateData(data, dataIndex, seriesScope);

Review comment:
       I'm thinking which scheme will be better here?
   
   Before
   ```js
   var symbolType = symbolEl._symbolType; // easy to get, but it is private, can I access it here?
   symbolType.indexOf('image://') === 0 && symbolEl.updateData(data, dataIndex, seriesScope);
   ```
   Now
   ```js
   var symbolType = symbolEl.getSymbolPath().type;
   symbolType === 'image' && symbolEl.updateData(data, dataIndex, seriesScope);
   ```




----------------------------------------------------------------
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 #12367: fix(tree): the symbols of image type do not display at the first rendering. close #12279.

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


   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