Added activeOn 'up/down' property

[melchiar]

Let me know if this is a feature you think this is worth merging. I've had comments from users who find the touch controls difficult since it's easy to accidentally drag/move an object while trying to select it. This property lets you change the touch behavior to select an object on the first touch, and only drag it on subsequent touches.

Not certain on the property name so let me know if you think of a better one.

[asturur]

I want to have this option, but not for touch only.
I tried to add a a property called activeOnMouseUp ( or pick up a better name ) in which the selection process would work only on mouseUp ( or touchup )

If you want to think at something like that, i would appreciate that.
That should be enabled on a per object basis, since it would allow for extra interactions and pseudo features if used in combination with other features.

[melchiar]

K, I've changed it to an object level property instead. I've used the name setActiveOn to allow individual control control over the mouse/touch behavior. Do these options make sense?

• When 'mousetouchdown', object is set to active on mousedown/touchdown
• When 'mousedowntouchup', object is set to active on mousedown/touchup
• When 'mouseuptouchdown', object is set to active on mouseup/touchdown
• When 'mousetouchup', object is set to active on mouseup/touchup

Github actions seems to work great btw!

[asturur]

Ok let's discuss it fully, there are a bunch of things i would like to talk about.
First thanks for trying this, i really want this feature.

1. should we offer 4 values? mouse/touch up/down with combinations? my first try would go with a simple up/down
2. touchstart/touchend instead of touchup touchdown would be better anyway.
   Apart from the decision on point 1) there is the interaction with grouping and shift click to talk about.
   if i have select on mouse up for example and i mouse down and start to drag on an object, what should happen? should i have an group selector that start to grow?

[melchiar]

Good points. Here are my thoughts.

1. If you'd prefer to keep it simple and have a single property for both mouse and touch I'm totally good with that (thinking about it, that would probably make the code easier to manage). For my own project I intend to have it behave differently with touch simply because that's the input where our own users struggle with accidental movements. Regardless I can always achieve my desired behavior by toggling this property with events so I'll defer to your judgement on this one.
2. If we take the approach of making this a simple boolean that affects mouse and touch equally, naming the property will be difficult since as you've pointed out, the touch equivalents of mousedown/mouseup are actually touchstart/touchend. Calling it activeOnMouseUpTouchEnd would be accurate but long. activeAfterClick perhaps?
3. If we make dragging on an object trigger the group selector, should we enforce a minimum drag distance before it triggers to avoid accidental group selections on touch devices?
4. Should we be considering how any changes we make here will impact future support for panning or gestures? I can imagine some devs wanting to swap the drag to group select functionality for panning, particularly if multiselect can be done with longpress or two-finger touch.

[asturur]

I would like to take a safe approach.
We add the property: activeOn: 'down'/'up'.
We complete it, we add a test, we give it a shot. We keep as @Private for now. We add a comment that we are still investigating it.
We merge it and we see how many things are broken or just became strange.
Then we reiterate on how we can improve the combination mouse/touch.
Does fabricJS offer a way to change the behaviour dinamically with an existing event ( like mouse:down:before )?
Is the generic experince good enough? does it allow for touch scrolling or does it allow to manipulate easily locked objects?

[melchiar]

Sounds good, let me know if you think these changes will work. I've taken the approach of simply checking for the activeOn === 'up' so that any invalid values will just revert to the default down behavior.

Yes, I would use the mouse:down:before event to toggle the property based on the event type so I'm very happy to do it this way. As for the behavior when dragging over an object prior to selecting, shall we leave that for now and examine it further after testing?

Also, what kind of test would you suggest adding for this?

[asturur]

can we specify === 'down' instead of 'up'?

[asturur]

How do we do for json dump on this? We often do not export events/interactions property. But i feel like this is different. I do not want to resolve this in this single PR. Just give it a thought

[melchiar]

sure, my thinking in doing it this way was that invalid values would just revert the behavior back to the default but I can change it if you don't think that's necessary

[melchiar]

my intention was to exclude this prop from the json since I feel it's not likely to be changed much and can always be manually included in the extraProperties array

[asturur]

Are you able to write a small test?
There should be some mouse down/up test already working and taking care of selections process

[melchiar]

sure, let me know if I did it correctly

[asturur]

Let me merge it, i'll play with it and figure out if we need some changes.
I want to keep this as small as possible and mixable with other behaviours.
This seems a good start.