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 2021/04/18 07:38:29 UTC

[GitHub] [echarts] villebro opened a new pull request #14702: fix(pie): hide labelLine emphasis when inside

villebro opened a new pull request #14702:
URL: https://github.com/apache/echarts/pull/14702


   ## Brief Information
   
   This pull request is in the type of:
   
   - [x] bug fixing
   - [ ] new feature
   - [ ] others
   
   ### What does this PR do?
   
   Currently the label line will stay visible on emphasis when changing from outside to inside. This PR fixes this problem and adds a test to show that emphasis works even after changing back and forth.
   
   
   ## Details
   
   ### Before: What was the problem?
   
   ![pie-before](https://user-images.githubusercontent.com/33317356/115137975-f3429200-a031-11eb-995f-38992b046006.gif)
   
   ### After: How is it fixed in this PR?
   
   ![pie-after](https://user-images.githubusercontent.com/33317356/115137982-fccbfa00-a031-11eb-803e-419befee11fa.gif)
   
   ### Merging options
   
   - [x] 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] [echarts] susiwen8 commented on pull request #14702: fix(pie): hide labelLine emphasis when inside

Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #14702:
URL: https://github.com/apache/echarts/pull/14702#issuecomment-822235463


   LGTM, @pissang could you double check?


-- 
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] [echarts] pissang commented on a change in pull request #14702: fix(pie): hide labelLine emphasis when inside

Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #14702:
URL: https://github.com/apache/echarts/pull/14702#discussion_r616327385



##########
File path: src/chart/pie/PieView.ts
##########
@@ -193,7 +193,7 @@ class PiePiece extends graphic.Sector {
 
         const labelPosition = seriesModel.get(['label', 'position']);
         if (labelPosition !== 'outside' && labelPosition !== 'outer') {
-            sector.getTextGuideLine()?.hide();
+            sector.removeTextGuideLine();
             return;
         }

Review comment:
       Seems the guideline can't be restored if the position is changed from 'inside' to 'outside' again. Perhaps we need to recreate the guideline here instead of in https://github.com/apache/echarts/blob/2c88dec314fb8260c6957341df174da8cdb9daea/src/chart/pie/PieView.ts#L49
   ```ts
   if (..)  { ... }
   else {
     let polyline = this.getTextGuideLine();
     if (!polyline) {
       polyline = new graphic.Polyline();
       this.setTextGuideLine(polyline);
     }
     // Default use item visual color
      setLabelLineStyle(this, getLabelLineStatesModels(itemModel), {
          stroke: visualColor,
          opacity: retrieve3(labelLineModel.get(['lineStyle', 'opacity']), visualOpacity, 1)
       });
   }
   ```
   
   




-- 
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] [echarts] villebro commented on a change in pull request #14702: fix(pie): hide labelLine emphasis when inside

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #14702:
URL: https://github.com/apache/echarts/pull/14702#discussion_r616339688



##########
File path: src/chart/pie/PieView.ts
##########
@@ -193,7 +193,7 @@ class PiePiece extends graphic.Sector {
 
         const labelPosition = seriesModel.get(['label', 'position']);
         if (labelPosition !== 'outside' && labelPosition !== 'outer') {
-            sector.getTextGuideLine()?.hide();
+            sector.removeTextGuideLine();
             return;
         }

Review comment:
       The relevant updated test:
   ![pie-after2](https://user-images.githubusercontent.com/33317356/115338749-26109580-a1ac-11eb-83bf-f0ed2b9bd40a.gif)
   




-- 
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] [echarts] echarts-bot[bot] commented on pull request #14702: fix(pie): hide labelLine emphasis when inside

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


   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/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



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

[GitHub] [echarts] pissang merged pull request #14702: fix(pie): hide labelLine emphasis when inside

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


   


-- 
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] [echarts] echarts-bot[bot] commented on pull request #14702: fix(pie): hide labelLine emphasis when inside

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


   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] [echarts] villebro commented on a change in pull request #14702: fix(pie): hide labelLine emphasis when inside

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #14702:
URL: https://github.com/apache/echarts/pull/14702#discussion_r616338867



##########
File path: src/chart/pie/PieView.ts
##########
@@ -193,7 +193,7 @@ class PiePiece extends graphic.Sector {
 
         const labelPosition = seriesModel.get(['label', 'position']);
         if (labelPosition !== 'outside' && labelPosition !== 'outer') {
-            sector.getTextGuideLine()?.hide();
+            sector.removeTextGuideLine();
             return;
         }

Review comment:
       Thanks for the review @pissang ! This change makes sense, and I did the proposed changes. I also added yet another flip-flop (inside/outside) to the test to make sure it can change back yet one more time, just in 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



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