link far end aligns with the mouse pointer position #469

antonioru · 2019-11-17T23:29:56Z

Checklist

The code has been run through pretty yarn run pretty
The tests pass on CircleCI
You have referenced the issue(s) or other PR(s) this fixes/relates-to
The PR Template has been filled out (see below)
Had a beer/coffee because you are awesome

What?

The DragNewLinkState.fireMouseMoved method was not taking into account the click starting position when dragging a new link, it just assumes it's the same point (x,y) of the link port. Sometimes, especially with big ports, it causes a slight mismatch between the link far end point and the mouse position as reported in the issue #467

How?

I saved the mouse starting position within the DragNewLinkState class itself.

Feel good image:

(Add your own one below :])

antonioru · 2019-11-18T00:03:04Z

@dylanvorster running yarn run pretty will "prettify" some files not related to the Pull Request, I can revert em to the previous state so the PR is clean but I'd wait your feedback.
cheers

…orm#467

dylanvorster · 2019-11-18T14:24:46Z

packages/react-diagrams-core/src/states/DragNewLinkState.ts

@@ -56,6 +60,9 @@ export class DragNewLinkState extends AbstractDisplacementState<DiagramEngine> {
 					this.link.setSourcePort(this.port);
 					this.engine.getModel().addLink(this.link);
 					this.port.reportPosition();
+
+					// save the mouse position for further precision in calculating the link's far-end point
+					this.startingPoint = new Point(event.event.clientX, event.event.clientY);


The AbstractDisplacementState already contains this data in the abstract class :)

The AbstractDisplacementState already contains this data in the abstract class :)

that's brilliant!
I'm going to update the PR, thank you for the tip ;)

@dylanvorster MR updated! ;)

mathiasbaldissera · 2019-11-19T00:27:19Z

I think it's not working 100%.
I Updated my personal State with your calcs and this became the result

antonioru · 2019-11-19T08:27:29Z

I think it's not working 100%.
I Updated my personal State with your calcs and this became the result

Thank you mathiasbaldissera for checking it out, lately I've updated the code by using the internal variables (initialX, initialY), so the method looks like this now:

    fireMouseMoved(event: AbstractDisplacementStateEvent): any {
		const pos = this.port.getPosition();
		const linkNextPosX = pos.x + (this.initialX - pos.x) + event.virtualDisplacementX;
		const linkNextPosY = pos.y + (this.initialY - pos.y) + event.virtualDisplacementY;

		this.link.getLastPoint().setPosition(linkNextPosX, linkNextPosY);
		this.engine.repaintCanvas();
	}

can you please confirm this is not working for you?

antonioru · 2019-11-19T08:53:18Z

Ok I've better looked into it (and I apologise I haven't done it before) @mathiasbaldissera is right, apparently it works most of the time:

but sometimes it doesn't:

I can't really understand why and I'll try to figure out as soon as possible...

any suggestion @dylanvorster ?

mathiasbaldissera · 2019-11-19T13:09:12Z

@antonioru Using the initialXRelative and initialYRelative less the canvas offset worked to me

		const pos = this.port.getPosition()
		const linkNextPosX =
			pos.x + (this.initialXRelative - this.engine.getModel().getOffsetX() - pos.x) + event.virtualDisplacementX
		const linkNextPosY =
			pos.y + (this.initialYRelative - this.engine.getModel().getOffsetY() - pos.y) + event.virtualDisplacementY

		this.link.getLastPoint().setPosition(linkNextPosX, linkNextPosY)

antonioru · 2019-11-19T13:57:43Z

const pos = this.port.getPosition()
		const linkNextPosX =
			pos.x + (this.initialXRelative - this.engine.getModel().getOffsetX() - pos.x) + event.virtualDisplacementX
		const linkNextPosY =
			pos.y + (this.initialYRelative - this.engine.getModel().getOffsetY() - pos.y) + event.virtualDisplacementY

		this.link.getLastPoint().setPosition(linkNextPosX, linkNextPosY)

It doesn't work for me, it doesn't show the link at all

…ore precise

antonioru · 2019-11-19T14:03:51Z

@mathiasbaldissera @dylanvorster

Ok, I've updated the PR by using the relative coordinates (initialXRelative, initialYRelative) instead of the absolute ones (initialX, initialY) and it works for be every time now.

Please Mathias let me know if you find any other problem.

The code I've updated is the following:

	/**
	 * Calculates the link's far-end point position on mouse move.
	 * In order to be as precise as possible the mouse initialXRelative & initialYRelative are taken into account.
	 */
	fireMouseMoved(event: AbstractDisplacementStateEvent): any {
		const pos = this.port.getPosition();
		const linkNextPosX = pos.x + (this.initialXRelative - pos.x) + event.virtualDisplacementX;
		const linkNextPosY = pos.y + (this.initialYRelative - pos.y) + event.virtualDisplacementY;

		this.link.getLastPoint().setPosition(linkNextPosX, linkNextPosY);
		this.engine.repaintCanvas();
	}

I hope it helps :)

Thank you guys, you've been really helpful
I sincerely hope to possibly improve the library

mathiasbaldissera · 2019-11-19T14:05:04Z

I hope it helps :)

If u use only the relative coordinates, and move the canvas (generating an offsetX and offsetY to the canvas) it'll bug, did u try it?

antonioru · 2019-11-19T14:07:04Z

let me k

If u use only the relative coordinates, and move the canvas (generating an offsetX and offsetY to the canvas) it'll bug, did u try it?

I see your point, so the engine offset must be taken into account as well ...
let me dig a bit more into it... I really can't understand why using the engine offset screws the whole thing in demos :\

antonioru · 2019-11-19T14:10:25Z

@mathiasbaldissera

this seems to finally work even when you move the canvas :)

	/**
	 * Calculates the link's far-end point position on mouse move.
	 * In order to be as precise as possible the mouse initialXRelative & initialYRelative are taken into account.
	 */
	fireMouseMoved(event: AbstractDisplacementStateEvent): any {
		const pos = this.port.getPosition();
		const engineOffsetX = this.engine.getModel().getOffsetX();
		const engineOffsetY = this.engine.getModel().getOffsetY();
		const linkNextPosX = pos.x - engineOffsetX + (this.initialXRelative - pos.x) + event.virtualDisplacementX;
		const linkNextPosY = pos.y - engineOffsetY + (this.initialYRelative - pos.y) + event.virtualDisplacementY;

		this.link.getLastPoint().setPosition(linkNextPosX, linkNextPosY);
		this.engine.repaintCanvas();
	}

mathiasbaldissera · 2019-11-19T14:12:33Z

@mathiasbaldissera

this seems to finally work even when you move the canvas :)

	/**
	 * Calculates the link's far-end point position on mouse move.
	 * In order to be as precise as possible the mouse initialXRelative & initialYRelative are taken into account.
	 */
	fireMouseMoved(event: AbstractDisplacementStateEvent): any {
		const pos = this.port.getPosition();
		const engineOffsetX = this.engine.getModel().getOffsetX();
		const engineOffsetY = this.engine.getModel().getOffsetY();
		const linkNextPosX = pos.x - engineOffsetX + (this.initialXRelative - pos.x) + event.virtualDisplacementX;
		const linkNextPosY = pos.y - engineOffsetY + (this.initialYRelative - pos.y) + event.virtualDisplacementY;

		this.link.getLastPoint().setPosition(linkNextPosX, linkNextPosY);
		this.engine.repaintCanvas();
	}

I'll try it at the afternoon (now i'ts 11:12 here :v)

antonioru · 2019-11-19T14:16:21Z

hehe, take your time mate, I'll update the PR in the meanwhile :)
(( It's 14:14 here, greetings from London :))

mathiasbaldissera · 2019-11-19T14:47:24Z

@antonioru 1 question. How did you exactly run the demos? Every time I try to run i got erros, and I need to test the changes into my own projects (I didn't find you on Facebook to ask this question hehehe :v)

antonioru · 2019-11-19T14:52:06Z

@antonioru 1 question. How did you exactly run the demos? Every time I try to run i got erros, and I need to test the changes into my own projects (I didn't find you on Facebook to ask this question hehehe :v)

I git cloned the repository locally on my computer
then cd react-diagrams (or the name you gave to your local repository)
npm install or yarn install
then cd packages/diagrams-demo-gallery
again npm install or yarn install
npm start or yarn start

i think it's all reported into the doc somewhere

…sition

mathiasbaldissera · 2019-11-19T16:55:38Z

@antonioru 1 question. How did you exactly run the demos? Every time I try to run i got erros, and I need to test the changes into my own projects (I didn't find you on Facebook to ask this question hehehe :v)

I git cloned the repository locally on my computer

then cd react-diagrams (or the name you gave to your local repository)

npm install or yarn install

then cd packages/diagrams-demo-gallery

again npm install or yarn install

npm start or yarn start

i think it's all reported into the doc somewhere

I really forgot to run the yarn install in the demo-gallery 🤦‍♂️ thx

mathiasbaldissera · 2019-11-19T16:56:47Z

hehe, take your time mate, I'll update the PR in the meanwhile :)
(( It's 14:14 here, greetings from London :))

It work perfectly now ;)
((now 13:56, greetings from Brazil 😄 )

antonioru · 2019-11-19T17:02:03Z

So it just needs to be merged :))

Thank you @mathiasbaldissera, you really helped me out

@dylanvorster

dylanvorster · 2019-11-19T18:32:09Z

what a journey xD

antonioru mentioned this pull request Nov 17, 2019

link far-end point doesn't align with the mouse position #467

Closed

antonioru changed the title ~~link far end alignes with the mouse pointer position, fixes #467~~ link far end alignes with the mouse pointer position Nov 17, 2019

antonioru changed the title ~~link far end alignes with the mouse pointer position~~ link far end aligns with the mouse pointer position Nov 17, 2019

link far end alignes with the mouse pointer position, fixes projectst…

a3ca75d

…orm#467

antonioru force-pushed the hotfix/new-link-far-end branch from a1f0d5b to a3ca75d Compare November 18, 2019 08:48

dylanvorster reviewed Nov 18, 2019

View reviewed changes

getting the initial click position from the AbstractDisplacementState

ea6130d

antonioru force-pushed the hotfix/new-link-far-end branch from 67a3227 to ea6130d Compare November 18, 2019 20:23

Using initialRelative coordinates when creating a new link makes it m…

6dd0677

…ore precise

antonioru force-pushed the hotfix/new-link-far-end branch from 3b70531 to 6dd0677 Compare November 19, 2019 14:03

keeping the engine offset into account when calculating a new link po…

ed63bf1

…sition

dylanvorster approved these changes Nov 19, 2019

View reviewed changes

dylanvorster merged commit a450c9b into projectstorm:master Nov 19, 2019

pierre-moire mentioned this pull request Nov 20, 2019

Wrong link appearance while creating a link if zoom is activated #470

Closed

mathiasbaldissera mentioned this pull request Nov 20, 2019

Fixing #470 #471

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

link far end aligns with the mouse pointer position #469

link far end aligns with the mouse pointer position #469

antonioru commented Nov 17, 2019 •

edited

Loading

antonioru commented Nov 18, 2019 •

edited

Loading

dylanvorster Nov 18, 2019

antonioru Nov 18, 2019

antonioru Nov 18, 2019

mathiasbaldissera commented Nov 19, 2019

antonioru commented Nov 19, 2019 •

edited

Loading

antonioru commented Nov 19, 2019 •

edited

Loading

mathiasbaldissera commented Nov 19, 2019 •

edited

Loading

antonioru commented Nov 19, 2019

antonioru commented Nov 19, 2019 •

edited

Loading

mathiasbaldissera commented Nov 19, 2019 •

edited

Loading

antonioru commented Nov 19, 2019

antonioru commented Nov 19, 2019

mathiasbaldissera commented Nov 19, 2019

antonioru commented Nov 19, 2019

mathiasbaldissera commented Nov 19, 2019 •

edited

Loading

antonioru commented Nov 19, 2019

mathiasbaldissera commented Nov 19, 2019 •

edited

Loading

mathiasbaldissera commented Nov 19, 2019

antonioru commented Nov 19, 2019

dylanvorster commented Nov 19, 2019

link far end aligns with the mouse pointer position #469

link far end aligns with the mouse pointer position #469

Conversation

antonioru commented Nov 17, 2019 • edited Loading

Checklist

What?

How?

Feel good image:

antonioru commented Nov 18, 2019 • edited Loading

dylanvorster Nov 18, 2019

Choose a reason for hiding this comment

antonioru Nov 18, 2019

Choose a reason for hiding this comment

antonioru Nov 18, 2019

Choose a reason for hiding this comment

mathiasbaldissera commented Nov 19, 2019

antonioru commented Nov 19, 2019 • edited Loading

antonioru commented Nov 19, 2019 • edited Loading

mathiasbaldissera commented Nov 19, 2019 • edited Loading

antonioru commented Nov 19, 2019

antonioru commented Nov 19, 2019 • edited Loading

mathiasbaldissera commented Nov 19, 2019 • edited Loading

antonioru commented Nov 19, 2019

antonioru commented Nov 19, 2019

mathiasbaldissera commented Nov 19, 2019

antonioru commented Nov 19, 2019

mathiasbaldissera commented Nov 19, 2019 • edited Loading

antonioru commented Nov 19, 2019

mathiasbaldissera commented Nov 19, 2019 • edited Loading

mathiasbaldissera commented Nov 19, 2019

antonioru commented Nov 19, 2019

dylanvorster commented Nov 19, 2019

antonioru commented Nov 17, 2019 •

edited

Loading

antonioru commented Nov 18, 2019 •

edited

Loading

antonioru commented Nov 19, 2019 •

edited

Loading

antonioru commented Nov 19, 2019 •

edited

Loading

mathiasbaldissera commented Nov 19, 2019 •

edited

Loading

antonioru commented Nov 19, 2019 •

edited

Loading

mathiasbaldissera commented Nov 19, 2019 •

edited

Loading

mathiasbaldissera commented Nov 19, 2019 •

edited

Loading

mathiasbaldissera commented Nov 19, 2019 •

edited

Loading