Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(graph): make graph node draggable in none and circular layout #15428

Merged
merged 15 commits into from
Feb 28, 2022

Conversation

kongmoumou
Copy link
Contributor

@kongmoumou kongmoumou commented Jul 29, 2021

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

make graph node draggable when layout: 'none' and layout: 'circular'

Fixed issues

Details

Before: What was the problem?

Currently, draggable only work in layout: 'force'

After: How is it fixed in this PR?

  • set node draggable
  • update node layout, then recalculate edge layout
  • update the graph

Misc

Related test cases or examples to use the new APIs

test/graph-draggable.html

Others

Merging options

  • Please squash the commits into a single one when merge.

Other information

@echarts-bot
Copy link

echarts-bot bot commented Jul 29, 2021

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

@pull-request-size pull-request-size bot added size/L and removed size/S labels Jul 31, 2021
src/chart/graph/circularLayoutHelper.ts Outdated Show resolved Hide resolved
@@ -333,6 +333,9 @@ class GraphNode {
// Used in traverse of Graph
__visited: boolean;

// NOTE: only use in circular layout
private _fixed: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently store in the fixed option of nodeModel

@@ -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([
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the circularLayout, perhaps using

node !== draggingNode

Can also do the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only automatically set circular layout for the node not dragged yet, so should check node's fixed state, not only the dragging node. 😂

@pissang pissang modified the milestones: 5.2, 5.3 Aug 6, 2021
@100pah
Copy link
Member

100pah commented Sep 29, 2021

@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?

@kongmoumou
Copy link
Contributor Author

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

@100pah
Copy link
Member

100pah commented Sep 29, 2021

In the demo above I just do something like:

Screen Shot 2021-09-29 at 3 21 16 PM

and

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

@kongmoumou
Copy link
Contributor Author

In the demo above I just do something like:

and

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

@kongmoumou kongmoumou requested a review from 100pah September 29, 2021 07:34
@alexresolute
Copy link

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

@pissang pissang modified the milestones: 5.3, 5.4 Feb 21, 2022
@apitts
Copy link

apitts commented Feb 22, 2022

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

Copy link
Member

@100pah 100pah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@100pah 100pah merged commit ff68ced into apache:next Feb 28, 2022
@echarts-bot
Copy link

echarts-bot bot commented Feb 28, 2022

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

@jayarjo
Copy link

jayarjo commented Mar 14, 2022

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

@kongmoumou
Copy link
Contributor Author

Maybe try this 😂 @jayarjo

echarts.getInstanceByDom($0)._chartsViews[0]._model.getData().getItemLayout(0) // dragged node index

@Ovilia Ovilia removed this from the 5.4 milestone Sep 1, 2022
@kongmoumou
Copy link
Contributor Author

Hi @Ovilia , is there any plan when to release this feature~😂

@Ovilia Ovilia added this to the 5.4.0 milestone Oct 19, 2022
@plainheart plainheart modified the milestones: 5.4.0, 5.4.1 Nov 11, 2022
@plainheart plainheart changed the title feat(graph): simple graph draggable. close #14510 feat(graph): make graph node draggable in none and circular layout Nov 11, 2022
@echarts-bot

This comment was marked as outdated.

@plainheart
Copy link
Member

@kongmoumou Thanks for your work! But we are sorry this feature was not mentioned in the release note of v5.4.0 and it can't work due to some unexpected reason. I've listed this feature in v5.4.1.

@kongmoumou
Copy link
Contributor Author

Thx a lot 🎉~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants