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/07/29 07:02:22 UTC

[GitHub] [echarts] kongmoumou opened a new pull request #15428: feat(graph): simple graph draggable. close #14510

kongmoumou opened a new pull request #15428:
URL: https://github.com/apache/echarts/pull/15428


   <!-- 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
   - [x] new feature
   - [ ] others
   
   
   
   ### What does this PR do?
   
   <!-- USE ONCE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
   make graph node draggable when `layout: 'none'`
   
   
   ### Fixed issues
   
   <!--
   - #xxxx: ...
   -->
   - #14510
   
   ## Details
   
   ### Before: What was the problem?
   
   <!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
   
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   
   Currently, `draggable` only work in `layout: 'force'`
   
   ### 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. -->
   
   - set node draggable
   - update node layout, then recalculate edge layout
   - update the graph
   
   ## Misc
   
   <!-- ADD RELATED ISSUE ID WHEN APPLICABLE -->
   
   - [ ] The API has been changed (apache/echarts-doc#xxx).
   - [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
   
   ### Related test cases or examples to use the new APIs
   
   test/graph-simple.html
   
   
   ## 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.

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/GraphView.ts
##########
@@ -131,21 +136,38 @@ class GraphView extends ChartView {
             const draggable = itemModel.get('draggable');
             if (draggable) {
                 el.on('drag', () => {
-                    if (forceLayout) {
-                        forceLayout.warmUp();
-                        !this._layouting
-                            && this._startForceLayoutIteration(forceLayout, layoutAnimation);
-                        forceLayout.setFixed(idx);
-                        // Write position back to layout
-                        data.setItemLayout(idx, [el.x, el.y]);
+                    switch (layout) {
+                        case 'force':
+                            forceLayout.warmUp();
+                            !this._layouting
+                                && this._startForceLayoutIteration(forceLayout, layoutAnimation);
+                            forceLayout.setFixed(idx);
+                            // Write position back to layout
+                            data.setItemLayout(idx, [el.x, el.y]);
+                            break;
+                        case 'circular':
+                            // mark node fixed
+                            node.getModel<GraphNodeItemOption>().mergeOption({fixed: true});

Review comment:
       Now store in the layout object




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on pull request #15428: feat(graph): simple graph draggable. close #14510

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






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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +177,58 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // init circular layout for
+            // 1. layout undefined node
+            // 2. not fixed node
+            (!node.getLayout() || !node.getModel<GraphNodeItemOption>().get('fixed'))
+            && node.setLayout([
                 r * Math.cos(angle) + cx,
                 r * Math.sin(angle) + cy
             ]);
             angle += radianHalf;
         });
     }
 };
+
+export function rotateNodeLabel(
+    node: GraphNode,
+    circularRotateLabel: boolean,
+    cx: number,
+    cy: number
+) {
+    const el = node.getGraphicEl() as Symbol;
+    // need to check if el exists. '-' value may not create node element.
+    if (!el) {
+        return;
+    }
+    const nodeModel = node.getModel<GraphNodeItemOption>();
+    let labelRotate = nodeModel.get(['label', 'rotate']) || 0;
+    const symbolPath = el.getSymbolPath();
+    if (circularRotateLabel) {
+        const pos = node.getLayout();
+        let rad = Math.atan2(pos[1] - cy, pos[0] - cx);
+        if (rad < 0) {
+            rad = Math.PI * 2 + rad;
+        }
+        const isLeft = pos[0] < cx;
+        if (isLeft) {
+            rad = rad - Math.PI;
+        }
+        const textPosition = isLeft ? 'left' as const : 'right' as const;
+
+        symbolPath.setTextConfig({
+            rotation: -rad,
+            position: textPosition,
+            origin: 'center'
+        });
+        const emphasisState = symbolPath.ensureState('emphasis');
+        zrUtil.extend(emphasisState.textConfig || (emphasisState.textConfig = {}), {
+            position: textPosition
+        });
+    }
+    else {

Review comment:
       Or maybe `graphHelper.ts` ?




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] apitts commented on pull request #15428: feat(graph): simple graph draggable. close #14510

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


   This is also an essential feature for me. If I can help in any way with this pull request please let me know!


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah edited a comment on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
100pah edited a comment on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929909936


   In the demo above I just do something like:
   
   <img width="978" alt="Screen Shot 2021-09-29 at 3 21 16 PM" src="https://user-images.githubusercontent.com/1956569/135221801-5081f97a-ce6d-43db-99fa-a1b1aaade3eb.png">
   
   and
   ```js
               const pointerVec = [draggingPointer[0] - cx, draggingPointer[1] - cy];
               pointerRad = Math.atan2(pointerVec[1], pointerVec[0]);
               const layout = [cx + r * Math.cos(pointerRad), cy + r * Math.sin(pointerRad)];
               draggingNode.setLayout(layout);
   ```


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah merged pull request #15428: feat(graph): simple graph draggable. close #14510

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


   


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou edited a comment on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
kongmoumou edited a comment on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929878984


   > An suggestion of circular layout dragging:
   > 
   > <img alt="Screen Shot 2021-09-29 at 12 52 12 PM" width="538" src="https://user-images.githubusercontent.com/1956569/135206144-d495f407-70c2-4aa1-9965-c81e88e111e5.png">
   > 
   > <img alt="Screen Shot 2021-09-29 at 12 52 18 PM" width="451" src="https://user-images.githubusercontent.com/1956569/135206159-e6d46d07-accc-48c7-ae99-5060088c8974.png">
   
   Now I use the mouse position to calculate node layout, it may have better ux πŸ˜‚ @100pah 


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on pull request #15428: feat(graph): simple graph draggable. close #14510

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


   Maybe try this πŸ˜‚ @jayarjo 
   
   ```js
   echarts.getInstanceByDom($0)._chartsViews[0]._model.getData().getItemLayout(0) // dragged node index
   ```


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] jayarjo edited a comment on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
jayarjo edited a comment on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-1066585580


   @pissang @kongmoumou how one is supposed to grab the new coordinates of the node, after it was dragged? I checked the source and I see that pre-defined event handlers for `drag` and `dragend` are forcefully wiped off.
   
   We need to grab new coordinates and sync them with the backend.


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -77,6 +79,25 @@ export function circularLayout(
         return;
     }
 
+    if (draggingNode) {
+        const [tempX, tempY] = draggingNode.getLayout();
+        const tan = (tempY - cy) / (tempX - cx);
+        const radian = -Math.atan(tan);
+
+        tempX > cx

Review comment:
       done~




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +184,51 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // auto circular layout for not dragged node
+            !node.getFixed() && node.setLayout([
                 r * Math.cos(angle) + cx,
                 r * Math.sin(angle) + cy
             ]);
             angle += radianHalf;
         });
     }
 };
+
+export function rotateNodeLabel(
+    node: GraphNode,
+    circularRotateLabel: boolean,
+    cx: number,
+    cy: number
+) {
+    const nodeModel = node.getModel<GraphNodeItemOption>();
+    const el = node.getGraphicEl() as Symbol;

Review comment:
       We need to check if `el` exists. `'-'` value may not create node element.




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/data/Graph.ts
##########
@@ -333,6 +333,9 @@ class GraphNode {
     // Used in traverse of Graph
     __visited: boolean;
 
+    // NOTE: only use in circular layout
+    private _fixed: boolean = false;

Review comment:
       Currently store in the `fixed` option of nodeModel




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/component/helper/RoamController.ts
##########
@@ -179,6 +179,14 @@ class RoamController extends Eventful<{
             return;
         }
 
+        let el = e.target;
+        while (el) {
+            if (el.draggable) {
+                return;
+            }
+            el = el.__hostTarget ? el.__hostTarget : el.parent;

Review comment:
       When dragging el's textContent, it will also trigger the pan effect which is unexpected. So I check if its parent is draggable.




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] alexresolute commented on pull request #15428: feat(graph): simple graph draggable. close #14510

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


   Hi, has there been any update on this? Dragging nodes without force mode enabled would be a very helpful feature!


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] jayarjo commented on pull request #15428: feat(graph): simple graph draggable. close #14510

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


   How one is supposed to grab the new coordinates of the node, after it was dragged? I checked the source and I see that pre-defined event handlers for `drag` and `dragend` are forcefully wiped 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.

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah commented on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
100pah commented on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929837724






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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah edited a comment on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
100pah edited a comment on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929905437


   @kongmoumou 
   
   > Now I use the mouse position to calculate node layout, it may have better ux 
   
   I have a try:
   
   Old: 
   https://echarts-try.surge.sh/graph-draggable-old/graph-draggable.html
   
   New:
   https://echarts-try.surge.sh/graph-draggable/graph-draggable.html
   
   I think the "New" one is better. But what's your opinion about 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.

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/GraphView.ts
##########
@@ -131,21 +136,38 @@ class GraphView extends ChartView {
             const draggable = itemModel.get('draggable');
             if (draggable) {
                 el.on('drag', () => {
-                    if (forceLayout) {
-                        forceLayout.warmUp();
-                        !this._layouting
-                            && this._startForceLayoutIteration(forceLayout, layoutAnimation);
-                        forceLayout.setFixed(idx);
-                        // Write position back to layout
-                        data.setItemLayout(idx, [el.x, el.y]);
+                    switch (layout) {
+                        case 'force':
+                            forceLayout.warmUp();
+                            !this._layouting
+                                && this._startForceLayoutIteration(forceLayout, layoutAnimation);
+                            forceLayout.setFixed(idx);
+                            // Write position back to layout
+                            data.setItemLayout(idx, [el.x, el.y]);
+                            break;
+                        case 'circular':
+                            // mark node fixed
+                            node.getModel<GraphNodeItemOption>().mergeOption({fixed: true});

Review comment:
       I think it will be better to set the `fixed` property in the layout object.
   
   ```ts
   node.setLayout({ fixed: true });
   ```




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -77,6 +79,18 @@ export function circularLayout(
         return;
     }
 
+    if (draggingNode) {
+        const [tempX, tempY] = draggingNode.getLayout();
+        const v = [tempX - cx, tempY - cy];
+        vec2.normalize(v, v);
+        vec2.scale(v, v, r);
+        draggingNode.setLayout([cx + v[0], cy + v[1]]);
+
+        const circularRotateLabel = seriesModel.get('layout') === 'circular'

Review comment:
       Seems 
   ```ts
   seriesModel.get('layout') === 'circular'
   ```
   this logic is unnecessary?
   

##########
File path: src/component/helper/RoamController.ts
##########
@@ -179,6 +179,14 @@ class RoamController extends Eventful<{
             return;
         }
 
+        let el = e.target;
+        while (el) {
+            if (el.draggable) {
+                return;
+            }
+            el = el.__hostTarget ? el.__hostTarget : el.parent;

Review comment:
       It can be written as:
   ```ts
   el = el.__hostTarget || el.parent;
   ```
   
   Also, I'm not sure about what's the scene of this logic and if it's correct.  `e.target.draggable` has been checked and return earlier.

##########
File path: src/chart/graph/GraphView.ts
##########
@@ -131,21 +136,38 @@ class GraphView extends ChartView {
             const draggable = itemModel.get('draggable');
             if (draggable) {
                 el.on('drag', () => {
-                    if (forceLayout) {
-                        forceLayout.warmUp();
-                        !this._layouting
-                            && this._startForceLayoutIteration(forceLayout, layoutAnimation);
-                        forceLayout.setFixed(idx);
-                        // Write position back to layout
-                        data.setItemLayout(idx, [el.x, el.y]);
+                    switch (layout) {
+                        case 'force':
+                            forceLayout.warmUp();
+                            !this._layouting
+                                && this._startForceLayoutIteration(forceLayout, layoutAnimation);
+                            forceLayout.setFixed(idx);
+                            // Write position back to layout
+                            data.setItemLayout(idx, [el.x, el.y]);
+                            break;
+                        case 'circular':
+                            // mark node fixed
+                            node.getModel<GraphNodeItemOption>().mergeOption({fixed: true});

Review comment:
       Needs to reset the fixed value after `dragend`

##########
File path: src/chart/graph/GraphView.ts
##########
@@ -131,21 +136,38 @@ class GraphView extends ChartView {
             const draggable = itemModel.get('draggable');
             if (draggable) {
                 el.on('drag', () => {
-                    if (forceLayout) {
-                        forceLayout.warmUp();
-                        !this._layouting
-                            && this._startForceLayoutIteration(forceLayout, layoutAnimation);
-                        forceLayout.setFixed(idx);
-                        // Write position back to layout
-                        data.setItemLayout(idx, [el.x, el.y]);
+                    switch (layout) {
+                        case 'force':
+                            forceLayout.warmUp();
+                            !this._layouting
+                                && this._startForceLayoutIteration(forceLayout, layoutAnimation);
+                            forceLayout.setFixed(idx);
+                            // Write position back to layout
+                            data.setItemLayout(idx, [el.x, el.y]);
+                            break;
+                        case 'circular':
+                            // mark node fixed
+                            node.getModel<GraphNodeItemOption>().mergeOption({fixed: true});

Review comment:
       I think it will be better to set the `fixed` property in the layout object.
   
   ```ts
   node.setLayout({ fixed: true });
   ```

##########
File path: src/component/helper/RoamController.ts
##########
@@ -179,6 +179,14 @@ class RoamController extends Eventful<{
             return;
         }
 
+        let el = e.target;
+        while (el) {
+            if (el.draggable) {
+                return;
+            }
+            el = el.__hostTarget ? el.__hostTarget : el.parent;

Review comment:
       Understand, https://github.com/apache/echarts/blob/472e72b6fde58a2ff9f14f698e3c197cb03ec917/src/component/helper/RoamController.ts#L177 this line of code can be removed.




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah commented on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
100pah commented on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929905437


   @kongmoumou 
   
   > Now I use the mouse position to calculate node layout, it may have better ux 
   
   I have a try:
   
   Old: 
   https://echarts-try.surge.sh/graph-draggable-old/graph-draggable.html
   
   New:
   https://echarts-try.surge.sh/graph-draggable/graph-draggable.html
   
   I think the "New" one is better. But what's your point about 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.

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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


   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.

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -77,6 +79,25 @@ export function circularLayout(
         return;
     }
 
+    if (draggingNode) {
+        const [tempX, tempY] = draggingNode.getLayout();
+        const tan = (tempY - cy) / (tempX - cx);
+        const radian = -Math.atan(tan);
+
+        tempX > cx

Review comment:
       There is a better way to fix the node on the circle edge.
   
   ```ts
   import * as vector from 'zrender/src/core/vector';
   // Vector from circle center to node.
   const v = [tempX - cx, tempY - cy];
   // Normalize the vector from circle center to node.
   vector.normalize(v, v);
   // Scale to circle radius
   vector.scale(v, v, r);
   // Calcualate the new position
   draggingNode.setLayout([cx + v[0], cy + v[1]]); 
   ```

##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +184,55 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // auto circular layout for not dragged node
+            !node.getFixed() && node.setLayout([

Review comment:
       For the circularLayout, perhaps using
   ```
   node !== draggingNode
   ```
   Can also do the same?

##########
File path: src/data/Graph.ts
##########
@@ -333,6 +333,9 @@ class GraphNode {
     // Used in traverse of Graph
     __visited: boolean;
 
+    // NOTE: only use in circular layout
+    private _fixed: boolean = false;

Review comment:
       I think there is a better place to store the `fixed` status rather than the very basic graph structure. In fact, `series-graph.node` has `fixed` option. But currently it's only for force layout.




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah edited a comment on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
100pah edited a comment on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929905437






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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +184,51 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // auto circular layout for not dragged node
+            !node.getFixed() && node.setLayout([
                 r * Math.cos(angle) + cx,
                 r * Math.sin(angle) + cy
             ]);
             angle += radianHalf;
         });
     }
 };
+
+export function rotateNodeLabel(
+    node: GraphNode,
+    circularRotateLabel: boolean,
+    cx: number,
+    cy: number
+) {
+    const nodeModel = node.getModel<GraphNodeItemOption>();
+    const el = node.getGraphicEl() as Symbol;

Review comment:
       We need to check if `el` exists. `'-'` value may not create node element.




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/GraphView.ts
##########
@@ -131,21 +136,38 @@ class GraphView extends ChartView {
             const draggable = itemModel.get('draggable');
             if (draggable) {
                 el.on('drag', () => {
-                    if (forceLayout) {
-                        forceLayout.warmUp();
-                        !this._layouting
-                            && this._startForceLayoutIteration(forceLayout, layoutAnimation);
-                        forceLayout.setFixed(idx);
-                        // Write position back to layout
-                        data.setItemLayout(idx, [el.x, el.y]);
+                    switch (layout) {
+                        case 'force':
+                            forceLayout.warmUp();
+                            !this._layouting
+                                && this._startForceLayoutIteration(forceLayout, layoutAnimation);
+                            forceLayout.setFixed(idx);
+                            // Write position back to layout
+                            data.setItemLayout(idx, [el.x, el.y]);
+                            break;
+                        case 'circular':
+                            // mark node fixed
+                            node.getModel<GraphNodeItemOption>().mergeOption({fixed: true});

Review comment:
       Needs to reset the fixed value after `dragend`




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/component/helper/RoamController.ts
##########
@@ -179,6 +179,14 @@ class RoamController extends Eventful<{
             return;
         }
 
+        let el = e.target;
+        while (el) {
+            if (el.draggable) {
+                return;
+            }
+            el = el.__hostTarget ? el.__hostTarget : el.parent;

Review comment:
       Understand, https://github.com/apache/echarts/blob/472e72b6fde58a2ff9f14f698e3c197cb03ec917/src/component/helper/RoamController.ts#L177 this line of code can be removed.




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +177,58 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // init circular layout for
+            // 1. layout undefined node
+            // 2. not fixed node
+            (!node.getLayout() || !node.getModel<GraphNodeItemOption>().get('fixed'))

Review comment:
       should better be `.get('fixed', true)`

##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +177,58 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // init circular layout for
+            // 1. layout undefined node
+            // 2. not fixed node
+            (!node.getLayout() || !node.getModel<GraphNodeItemOption>().get('fixed'))
+            && node.setLayout([
                 r * Math.cos(angle) + cx,
                 r * Math.sin(angle) + cy
             ]);
             angle += radianHalf;
         });
     }
 };
+
+export function rotateNodeLabel(
+    node: GraphNode,
+    circularRotateLabel: boolean,
+    cx: number,
+    cy: number
+) {
+    const el = node.getGraphicEl() as Symbol;
+    // need to check if el exists. '-' value may not create node element.
+    if (!el) {
+        return;
+    }
+    const nodeModel = node.getModel<GraphNodeItemOption>();
+    let labelRotate = nodeModel.get(['label', 'rotate']) || 0;
+    const symbolPath = el.getSymbolPath();
+    if (circularRotateLabel) {
+        const pos = node.getLayout();
+        let rad = Math.atan2(pos[1] - cy, pos[0] - cx);
+        if (rad < 0) {
+            rad = Math.PI * 2 + rad;
+        }
+        const isLeft = pos[0] < cx;
+        if (isLeft) {
+            rad = rad - Math.PI;
+        }
+        const textPosition = isLeft ? 'left' as const : 'right' as const;
+
+        symbolPath.setTextConfig({
+            rotation: -rad,
+            position: textPosition,
+            origin: 'center'
+        });
+        const emphasisState = symbolPath.ensureState('emphasis');
+        zrUtil.extend(emphasisState.textConfig || (emphasisState.textConfig = {}), {
+            position: textPosition
+        });
+    }
+    else {

Review comment:
       This logic do not only related to 'circular layout'. All other layout use it.
   Should we put this logic in file `circularLayoutHelper.ts`? 




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on pull request #15428: feat(graph): simple graph draggable. close #14510

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


   > In the demo above I just do something like:
   >
   > and
   > 
   > ```js
   >             const pointerVec = [draggingPointer[0] - cx, draggingPointer[1] - cy];
   >             pointerRad = Math.atan2(pointerVec[1], pointerVec[0]);
   >             const layout = [cx + r * Math.cos(pointerRad), cy + r * Math.sin(pointerRad)];
   >             draggingNode.setLayout(layout);
   > ```
   
   Thx~ I use the same approach, u could review the latest 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.

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -77,6 +79,18 @@ export function circularLayout(
         return;
     }
 
+    if (draggingNode) {
+        const [tempX, tempY] = draggingNode.getLayout();
+        const v = [tempX - cx, tempY - cy];
+        vec2.normalize(v, v);
+        vec2.scale(v, v, r);
+        draggingNode.setLayout([cx + v[0], cy + v[1]]);
+
+        const circularRotateLabel = seriesModel.get('layout') === 'circular'

Review comment:
       Seems 
   ```ts
   seriesModel.get('layout') === 'circular'
   ```
   this logic is unnecessary?
   

##########
File path: src/component/helper/RoamController.ts
##########
@@ -179,6 +179,14 @@ class RoamController extends Eventful<{
             return;
         }
 
+        let el = e.target;
+        while (el) {
+            if (el.draggable) {
+                return;
+            }
+            el = el.__hostTarget ? el.__hostTarget : el.parent;

Review comment:
       It can be written as:
   ```ts
   el = el.__hostTarget || el.parent;
   ```
   
   Also, I'm not sure about what's the scene of this logic and if it's correct.  `e.target.draggable` has been checked and return earlier.




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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 #15428: feat(graph): simple graph draggable. close #14510

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


   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).


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +184,51 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // auto circular layout for not dragged node
+            !node.getFixed() && node.setLayout([
                 r * Math.cos(angle) + cx,
                 r * Math.sin(angle) + cy
             ]);
             angle += radianHalf;
         });
     }
 };
+
+export function rotateNodeLabel(
+    node: GraphNode,
+    circularRotateLabel: boolean,
+    cx: number,
+    cy: number
+) {
+    const nodeModel = node.getModel<GraphNodeItemOption>();
+    const el = node.getGraphicEl() as Symbol;

Review comment:
       done🀣




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah commented on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
100pah commented on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929837724


   An suggestion of circular layout dragging:
   
   <img width="538" alt="Screen Shot 2021-09-29 at 12 52 12 PM" src="https://user-images.githubusercontent.com/1956569/135206144-d495f407-70c2-4aa1-9965-c81e88e111e5.png">
   
   <img width="451" alt="Screen Shot 2021-09-29 at 12 52 18 PM" src="https://user-images.githubusercontent.com/1956569/135206159-e6d46d07-accc-48c7-ae99-5060088c8974.png">
   


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] jayarjo edited a comment on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
jayarjo edited a comment on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-1066585580


   @pissang @kongmoumou how one is supposed to grab the new coordinates of the node, after it was dragged? I checked the source and I see that pre-defined event handlers for `drag` and `dragend` are forcefully wiped 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.

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -77,6 +79,18 @@ export function circularLayout(
         return;
     }
 
+    if (draggingNode) {
+        const [tempX, tempY] = draggingNode.getLayout();
+        const v = [tempX - cx, tempY - cy];
+        vec2.normalize(v, v);
+        vec2.scale(v, v, r);
+        draggingNode.setLayout([cx + v[0], cy + v[1]]);
+
+        const circularRotateLabel = seriesModel.get('layout') === 'circular'

Review comment:
       done




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on pull request #15428: feat(graph): simple graph draggable. close #14510

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


   > An suggestion of circular layout dragging:
   > 
   > <img alt="Screen Shot 2021-09-29 at 12 52 12 PM" width="538" src="https://user-images.githubusercontent.com/1956569/135206144-d495f407-70c2-4aa1-9965-c81e88e111e5.png">
   > 
   > <img alt="Screen Shot 2021-09-29 at 12 52 18 PM" width="451" src="https://user-images.githubusercontent.com/1956569/135206159-e6d46d07-accc-48c7-ae99-5060088c8974.png">
   
   Now I use the mouse position to calculate node layout, it may have better ux πŸ˜‚


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on pull request #15428: feat(graph): simple graph draggable. close #14510

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


   > @kongmoumou
   > 
   > > Now I use the mouse position to calculate node layout, it may have better ux
   > 
   > I have a try:
   > 
   > Old: https://echarts-try.surge.sh/graph-draggable-old/graph-draggable.html
   > 
   > New: https://echarts-try.surge.sh/graph-draggable/graph-draggable.html
   > 
   > I think the "New" one is better. But what's your point about it?
   
   I agree with u πŸ˜‚, the "new" one is more controllable.


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +184,55 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // auto circular layout for not dragged node
+            !node.getFixed() && node.setLayout([

Review comment:
       Only automatically set circular layout for the node not dragged yet, so should check node's fixed state, not only the dragging node. πŸ˜‚




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou edited a comment on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
kongmoumou edited a comment on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929878984


   > An suggestion of circular layout dragging:
   > 
   > <img alt="Screen Shot 2021-09-29 at 12 52 12 PM" width="538" src="https://user-images.githubusercontent.com/1956569/135206144-d495f407-70c2-4aa1-9965-c81e88e111e5.png">
   > 
   > <img alt="Screen Shot 2021-09-29 at 12 52 18 PM" width="451" src="https://user-images.githubusercontent.com/1956569/135206159-e6d46d07-accc-48c7-ae99-5060088c8974.png">
   
   Now I use the mouse position to calculate node layout, it may have better ux πŸ˜‚ @100pah 


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah commented on pull request #15428: feat(graph): simple graph draggable. close #14510

Posted by GitBox <gi...@apache.org>.
100pah commented on pull request #15428:
URL: https://github.com/apache/echarts/pull/15428#issuecomment-929909936


   In the demo above I just do something like:
   
   <img width="978" alt="Screen Shot 2021-09-29 at 3 21 16 PM" src="https://user-images.githubusercontent.com/1956569/135221801-5081f97a-ce6d-43db-99fa-a1b1aaade3eb.png">
   


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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] kongmoumou commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/component/helper/RoamController.ts
##########
@@ -179,6 +179,14 @@ class RoamController extends Eventful<{
             return;
         }
 
+        let el = e.target;
+        while (el) {
+            if (el.draggable) {
+                return;
+            }
+            el = el.__hostTarget ? el.__hostTarget : el.parent;

Review comment:
       When dragging el's textContent, it will also trigger the pan effect which is unexpected. So I check if its parent is draggable.

##########
File path: src/chart/graph/GraphView.ts
##########
@@ -131,21 +136,38 @@ class GraphView extends ChartView {
             const draggable = itemModel.get('draggable');
             if (draggable) {
                 el.on('drag', () => {
-                    if (forceLayout) {
-                        forceLayout.warmUp();
-                        !this._layouting
-                            && this._startForceLayoutIteration(forceLayout, layoutAnimation);
-                        forceLayout.setFixed(idx);
-                        // Write position back to layout
-                        data.setItemLayout(idx, [el.x, el.y]);
+                    switch (layout) {
+                        case 'force':
+                            forceLayout.warmUp();
+                            !this._layouting
+                                && this._startForceLayoutIteration(forceLayout, layoutAnimation);
+                            forceLayout.setFixed(idx);
+                            // Write position back to layout
+                            data.setItemLayout(idx, [el.x, el.y]);
+                            break;
+                        case 'circular':
+                            // mark node fixed
+                            node.getModel<GraphNodeItemOption>().mergeOption({fixed: true});

Review comment:
       Now store in the layout object

##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -77,6 +79,18 @@ export function circularLayout(
         return;
     }
 
+    if (draggingNode) {
+        const [tempX, tempY] = draggingNode.getLayout();
+        const v = [tempX - cx, tempY - cy];
+        vec2.normalize(v, v);
+        vec2.scale(v, v, r);
+        draggingNode.setLayout([cx + v[0], cy + v[1]]);
+
+        const circularRotateLabel = seriesModel.get('layout') === 'circular'

Review comment:
       done

##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +177,58 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // init circular layout for
+            // 1. layout undefined node
+            // 2. not fixed node
+            (!node.getLayout() || !node.getModel<GraphNodeItemOption>().get('fixed'))
+            && node.setLayout([
                 r * Math.cos(angle) + cx,
                 r * Math.sin(angle) + cy
             ]);
             angle += radianHalf;
         });
     }
 };
+
+export function rotateNodeLabel(
+    node: GraphNode,
+    circularRotateLabel: boolean,
+    cx: number,
+    cy: number
+) {
+    const el = node.getGraphicEl() as Symbol;
+    // need to check if el exists. '-' value may not create node element.
+    if (!el) {
+        return;
+    }
+    const nodeModel = node.getModel<GraphNodeItemOption>();
+    let labelRotate = nodeModel.get(['label', 'rotate']) || 0;
+    const symbolPath = el.getSymbolPath();
+    if (circularRotateLabel) {
+        const pos = node.getLayout();
+        let rad = Math.atan2(pos[1] - cy, pos[0] - cx);
+        if (rad < 0) {
+            rad = Math.PI * 2 + rad;
+        }
+        const isLeft = pos[0] < cx;
+        if (isLeft) {
+            rad = rad - Math.PI;
+        }
+        const textPosition = isLeft ? 'left' as const : 'right' as const;
+
+        symbolPath.setTextConfig({
+            rotation: -rad,
+            position: textPosition,
+            origin: 'center'
+        });
+        const emphasisState = symbolPath.ensureState('emphasis');
+        zrUtil.extend(emphasisState.textConfig || (emphasisState.textConfig = {}), {
+            position: textPosition
+        });
+    }
+    else {

Review comment:
       Or maybe `graphHelper.ts` ?




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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] 100pah commented on a change in pull request #15428: feat(graph): simple graph draggable. close #14510

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



##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +177,58 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // init circular layout for
+            // 1. layout undefined node
+            // 2. not fixed node
+            (!node.getLayout() || !node.getModel<GraphNodeItemOption>().get('fixed'))

Review comment:
       should better be `.get('fixed', true)`

##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -163,11 +177,58 @@ const _layoutNodesBasedOn: Record<'value' | 'symbolSize', LayoutNode> = {
             const radianHalf = halfRemainRadian + _symbolRadiansHalf[node.dataIndex];
 
             angle += radianHalf;
-            node.setLayout([
+            // init circular layout for
+            // 1. layout undefined node
+            // 2. not fixed node
+            (!node.getLayout() || !node.getModel<GraphNodeItemOption>().get('fixed'))
+            && node.setLayout([
                 r * Math.cos(angle) + cx,
                 r * Math.sin(angle) + cy
             ]);
             angle += radianHalf;
         });
     }
 };
+
+export function rotateNodeLabel(
+    node: GraphNode,
+    circularRotateLabel: boolean,
+    cx: number,
+    cy: number
+) {
+    const el = node.getGraphicEl() as Symbol;
+    // need to check if el exists. '-' value may not create node element.
+    if (!el) {
+        return;
+    }
+    const nodeModel = node.getModel<GraphNodeItemOption>();
+    let labelRotate = nodeModel.get(['label', 'rotate']) || 0;
+    const symbolPath = el.getSymbolPath();
+    if (circularRotateLabel) {
+        const pos = node.getLayout();
+        let rad = Math.atan2(pos[1] - cy, pos[0] - cx);
+        if (rad < 0) {
+            rad = Math.PI * 2 + rad;
+        }
+        const isLeft = pos[0] < cx;
+        if (isLeft) {
+            rad = rad - Math.PI;
+        }
+        const textPosition = isLeft ? 'left' as const : 'right' as const;
+
+        symbolPath.setTextConfig({
+            rotation: -rad,
+            position: textPosition,
+            origin: 'center'
+        });
+        const emphasisState = symbolPath.ensureState('emphasis');
+        zrUtil.extend(emphasisState.textConfig || (emphasisState.textConfig = {}), {
+            position: textPosition
+        });
+    }
+    else {

Review comment:
       This logic do not only related to 'circular layout'. All other layout use it.
   Should we put this logic in file `circularLayoutHelper.ts`? 




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

To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org

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