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

Lock and controls (drawing+cursors) #3712

Closed
ncou opened this issue Feb 17, 2017 · 13 comments · Fixed by #4064 or #4067
Closed

Lock and controls (drawing+cursors) #3712

ncou opened this issue Feb 17, 2017 · 13 comments · Fixed by #4064 or #4067
Labels
Milestone

Comments

@ncou
Copy link
Collaborator

ncou commented Feb 17, 2017

Hi,

Just my 2 cents idear, but perhaps it could became a new feature.

When you set "lockRotation : true" the rotation is locked but the "rect" control is still present/draw.
ofcourse there is the "hasRotationPoint" propertie, but "lockUniScaling" disable the drawing of some controls rect.
1)
It could be interesting to not draw the rotation rect control if hasRotation=false OR lockRotation

When the scalingX&Y or MovementX&Y is set to true, the cursors (to stretch or move) are still displayed it's a bit disturbing to have the cursor while the action is disabled.
Perhaps those tests could be added in the fabric.js code, to disable cursors change ?

@asturur : What do you think ?

@asturur
Copy link
Member

asturur commented Feb 18, 2017

i think that this is right.
i have to verify them all but yes. for embedded functions there should be consistency.

@asturur
Copy link
Member

asturur commented Feb 19, 2017

so the idea is this:

lockRotation, lockUniscaling, lockMovement will LOCK the action, disable the cursor that tells you the action is available.

they will not disable the corners since this is weird.

Since this is a behaviour change, it will go just in fabric 2.

@asturur
Copy link
Member

asturur commented Feb 19, 2017

maybe introducing the notAllowed cursor as not-allowed css feature

@asturur asturur added this to the 2.0.0 milestone Feb 19, 2017
@asturur asturur changed the title [Feature request] Lock and controls (drawing+cursors) Lock and controls (drawing+cursors) Feb 19, 2017
@ncou
Copy link
Collaborator Author

ncou commented Feb 20, 2017

yes it could be great to have a propertie "notAllowedCursor" cursor for the locked actions it's would be more clear for the user when the object transformations are locked.

But perhaps (when the lockMovementX&Y are set) this locked cursor should not overide the "hoverCursor" defined, because it would be strange for the users to have a locked cursor on hover while we can still select the object. Of course the "moveCursor" would be overriden by the "notAllowedCursor".

Not sure if i am clear enough. In any case it's a good feature for the v2.0.0 :)

@ncou
Copy link
Collaborator Author

ncou commented Jul 3, 2017

@asturur : thank you, i will do some testing. By the way i tryed to build the library from the master, but the build.js throw a missing file "named_accessors.js" in the src\util folder.
The file is missing there is only a "named_accessors.mixin.js" file, I renamed it and the build is now ok.

@asturur
Copy link
Member

asturur commented Jul 3, 2017 via email

@ncou
Copy link
Collaborator Author

ncou commented Jul 3, 2017

the PR is done.
Yes i use the string "node build.js modules=ALL" to build the lib.

@ncou
Copy link
Collaborator Author

ncou commented Jul 3, 2017

@asturur : i think there is a small bug.
http://jsfiddle.net/qu96G/20/

If i lock the scalingX or scalingY, the middle controls are correctly locked, but not the corner controls.
I can't stretch the object using the corner controls, but the cursor is not "not-allowed".
Disable line 38 the "lockUniScaling" to have the centered controls with the correct notallowedcursor.

Question :
If I lock the movementX + movementY, shouldn't the hoverCusor be overrided by the "notAllowedCursor" ? my object can't move, but the cursor is still the moving cursor.
=> Perhaps it's a border use case.

PS : If i select multiple object, the lock properties is ignored, so the not-allowed cursor is not displayed. But there is a ticket about this.

@asturur
Copy link
Member

asturur commented Jul 3, 2017

to answer:

for movementX, movementY i opened a new feature ticket.

for the lock i did not rememeber that lockScalingX was killing the corner scaling! i ll fix it.

lockuniscaling is something to be checked. if is disabling controls, i need to modify it also.

@asturur asturur reopened this Jul 3, 2017
@asturur
Copy link
Member

asturur commented Jul 3, 2017

For the group, just to explain a bit:

Group is not gonna check children properties for lock* anymore. It will respect just its own properties.

A new class will be created, called ActiveGroup that will be behave as a multiSelection. Group will be more a simple container with little features, the ActiveGroup will inherit from Group and do the things a user maybe expect.

Also creating a new ActiveGroup should also solve the headaches of creating a multi selectin programmatically.

@ncou
Copy link
Collaborator Author

ncou commented Jul 5, 2017

Does this mean the logic to "lock" the group based on the "children-objects" properties, will be already coded in the future "ActiveGroup" object ? or does this need to be developped by each developer ?

Will it be possible to implement our own rules to avoid a selection ? for example if i don't want an object being added in the "ActiveGroup" selection (for example because it's a locked object), could i implement this behavior ?

@asturur
Copy link
Member

asturur commented Jul 5, 2017

Yeah this is the right moment to rise those questions.
I ll take in consideration this thing while writing, to see if it can fitted and where.
(if this.selectabled does not work.... )

@asturur
Copy link
Member

asturur commented Jul 5, 2017

i did this so far. #4076 not working yet
i ll continue tomorrow.

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

Successfully merging a pull request may close this issue.

2 participants