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

adding object properties of snapAngle and snapThreshold to enable object snapping. #3383

Merged
merged 3 commits into from
Oct 29, 2016

Conversation

stefanhayden
Copy link
Member

@stefanhayden stefanhayden commented Oct 28, 2016

Adds a way to set an angle interval to snap to during rotation (snapAngle) and the ability to set the threshold of how far away it snaps to that angle (snapThreshold).

When the snapThreshold is null (the default) it will use the snapAngle until the user sets the snapThreshold. This is good because it makes the api easier to use and also setting the snapAngle will feel more obvious with the threshold and the angle the same.

Here is an example of just the snapAngle set toe 45:
fabric

@asturur mentioned adding a shift or alt modifier key to turn the behavior off but I think that might be better handled in code like so:

rect.on('rotating', function(options) {
  if(options.e.shiftKey) {
    this.snapAngle = 0;
  } else {
    this.snapAngle = 45;
  }
});

To me this keep fabric feeling more like a tool, while assigning predetermined modifier keys to disable behavior makes the library feel more opinionated then a tool should be. This code to disable the snap behavior could easily go in a tutorial somewhere.

close #3369

@stefanhayden stefanhayden changed the title adding object properties of and to enable object snapping. adding object properties of snapAngle and snapThreshold to enable object snapping. Oct 28, 2016
@asturur
Copy link
Member

asturur commented Oct 28, 2016

Hello Stefan, thank for bringing this to the finish line.

There is some linting failing:
974:1 error Trailing spaces not allowed no-trailing-spaces 981:1 error Trailing spaces not allowed no-trailing-spaces 984:14 error Closing curly brace appears on the same line as the subsequent block brace-style

Also we have sort of convention to group sequential var declaration ( first this was lintend, now no more, i think i lost the rule in a conversion process )

so

+      var snapAngle  = t.target.snapAngle;
+      var snapThreshold  = t.target.snapThreshold || snapAngle;
+      var rightAngleLocked = Math.ceil(angle / snapAngle) * snapAngle;
+      var leftAngleLocked = Math.floor(angle / snapAngle) * snapAngle;

should be

+      var snapAngle  = t.target.snapAngle,
+          snapThreshold  = t.target.snapThreshold || snapAngle,
+          rightAngleLocked = Math.ceil(angle / snapAngle) * snapAngle,
+          leftAngleLocked = Math.floor(angle / snapAngle) * snapAngle;

@asturur
Copy link
Member

asturur commented Oct 28, 2016

a couple of things come to my mind.
Is not documented anywhere if not reading all the transformation logic, but that function should return TRUE only if a rotation happened, otherwise a fake object modified event would fire.
At this point we should still do 2 things:

  1. execute all the math about snapping only if snapAngle is > 0
  2. in case of snapping, return true only if the angle changed.

sorry for not bringing this up before.

…true if a rotation angle changed has happened.
@stefanhayden
Copy link
Member Author

good thoughts. updated!

@asturur
Copy link
Member

asturur commented Oct 29, 2016

i think we are good to go.

@asturur asturur merged commit 1070305 into fabricjs:master Oct 29, 2016
asturur pushed a commit that referenced this pull request Oct 29, 2016
…ect snapping. (#3383)

* adding object properties of  and  to enable object snapping.
@ncou
Copy link
Collaborator

ncou commented Nov 13, 2016

Hi,

Nice feature, i tried it on a fiddle : http://jsfiddle.net/pec86/128/
But it seems to me there is a sligth glitch when i made a manual rotation followed by a snapped Rotation.

For example if i have set the snap threshold at 10° : If I made a manual rotation at 13.5°(it's the last value show in the console, then clear the console) after i made a snapped rotation (by pressing the shift key, and I continue the rotation), the angle show in the console is first 13.6° then 10°.

Looks like there is a first setAngle called but without the snapped value. So it's result with a sligth forward&backward movement for the object.

Can you confirm it ?

Keep up the good work.

@stefanhayden
Copy link
Member Author

I guess the real issue is with the example. The rotation event is called after rotation has happened so if you wait for rotation to set the snapAngle then you get a single tick of a normal angle. The solution is to set the snapAngle on shift keydown / keyup events.

Here is your jsfiddle updated to get rid of the extra rotation tick before the snap: http://jsfiddle.net/yvo89upg/

and here is the bulk of the change in the jsfiddle:

var body = document.querySelector('body');
body.addEventListener('keydown', function(e) {
    var fabricObject = canvas.getActiveGroup() || canvas.getActiveObject();
  if (fabricObject && e.shiftKey) {
    fabricObject.snapAngle = 10;
  }
});
body.addEventListener('keyup', function(e) {
    var fabricObject = canvas.getActiveGroup() || canvas.getActiveObject();
  if (fabricObject && !e.shiftKey) {
    fabricObject.snapAngle = 0;
  }
});

canvas.on('object:rotating', function(options) {  
  console.log(options.target.angle);
});

@ncou
Copy link
Collaborator

ncou commented Nov 13, 2016

Thank you, i understand better how to use this. hope this will go in the documentation. have a good day

@stevenkaspar
Copy link

That jsfiddle didn't work for me. It was saying canvas.getActiveGroup() wasn't a function

I updated it and used canvas.getActiveObjects()

body.addEventListener('keydown', function(e) {
  var fabricObjects = canvas.getActiveObjects();
  if (fabricObjects && e.shiftKey) {
    fabricObjects.forEach(function(o){
    	o.snapAngle = 15;
    })
  }
});
body.addEventListener('keyup', function(e) {
  var fabricObjects = canvas.getActiveObjects();
  if (fabricObjects && !e.shiftKey) {
    fabricObjects.forEach(function(o){
    	o.snapAngle = 0;
    })
  }
});

http://jsfiddle.net/stevenkaspar/x51r2d0m/

@ShaMan123 ShaMan123 mentioned this pull request Oct 29, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Rotation snap
4 participants