Skip to content

Conversation

jameszhong2008
Copy link

Description

Hello, since fabric 6, fabric support Group better. But it can not select object in group on UI.
I add a option 'dblClickIsolateObject' in CanvasOptions.

Turn on this option by

new fabric.Canvas(ele, {
        dblClickIsolateObject: true,
  }
);

When double click on a group, we can select the children of this group.

In Action

image image

Copy link

codesandbox bot commented Sep 21, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

* @private
*/
declare private _willAddMouseDown: number;
private declare _willAddMouseDown: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

this change doesn't seem relevant to the feature you added

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this is not relevant to the feature, it was format automatically when saving. it is default format after typescript 4.0.
If this doesn't fit, I'll restore it.

Copy link
Contributor

@Smrtnyk Smrtnyk Sep 21, 2025

Choose a reason for hiding this comment

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

I would refrain from making any changes that are unrelated to the problem you are trying to solve
just adds extra burden to the reviewers and makes the diff larger

@Smrtnyk
Copy link
Contributor

Smrtnyk commented Sep 21, 2025

was this feature discussed before as acceptable, before you implemented it? is there an issue you can link to?
also you added this feature but not a single test, either unit or visual ones

@jameszhong2008
Copy link
Author

was this feature discussed before as acceptable, before you implemented it? is there an issue you can link to? also you added this feature but not a single test, either unit or visual ones

Our graphic editor based fabric uses a tree structure and needs to modify child nodes directly. So we implemented it, and I think this feature will be useful to others.

@Smrtnyk
Copy link
Contributor

Smrtnyk commented Sep 21, 2025

was this feature discussed before as acceptable, before you implemented it? is there an issue you can link to? also you added this feature but not a single test, either unit or visual ones

Our graphic editor based fabric uses a tree structure and needs to modify child nodes directly. So we implemented it, and I think this feature will be useful to others.

@asturur has the last word here
but in case he rejects the feature you then just waisted your time implementing this
that is why it is better to first open a feature request or open a discussion

@asturur
Copy link
Member

asturur commented Sep 21, 2025

We can't insert every feature that every editor wants.
Many features create an exponential number of cases you have to maintain with all permutation and combinations of features, but on top of everything this feature happens on double click you can implement it as an event handler on the dblclick.
I understand that some people would find this already done and would be happy, but many others could want it slightly different and would be locked into this implementation.
It would be useful if you could turn this code into an event driven feature and make a tutorial out of it on the website, as a feature i can't accept it.

@asturur
Copy link
Member

asturur commented Sep 21, 2025

The point made by @Smrtnyk is correct. If you want to contribute discuss the feature first and see what are the options to make your feature available to the community. The projects needs documentation above anything else, examples, explanations, tutorials. Adding features is not really convenient at this stage

@jameszhong2008
Copy link
Author

OK,I'll find a better way. About event driven feature, can you give me more detail. I'm not familiar with it.

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.

3 participants