search

fabricjs / fabric.js

Javascript Canvas Library, SVG-to-Canvas (& canvas-to-SVG) Parser
http://fabricjs.com
Other
28.82k stars 3.5k forks source link

fixed renderControls order #9914

Closed DaxGama closed 3 months ago

DaxGama commented 3 months ago

Fixed controls render order for ActiveSelection

Before Child controls renders over ActiveSelection

image

After ActiveSelection controls renders over child's

image

This is my first contribution to this project, please let me know if I have missed out anything.

codesandbox[bot] commented 3 months ago

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders
Open Preview

asturur commented 3 months ago

This is a bit of really a preference issue. In which case do we have both group and child selected? that is the first thing to understand. It shouldn't happen in theory.

The screenshot doesn't really give out many informations, could you make a small example with a code snippet of what are you running and how you ended up in a situation that didn't make sense to you?

DaxGama commented 3 months ago

HI @asturur I realised I did't descirbe it well. This is the logic for ActiveSelection not group.

The scenario is simple, when ever you select multiple objects, the borders of _objects is drawn over the border of selection.

The _objects are explicitly set by code to not render controls, so there won't ever be a scenario where conrols of _objects are render under the active selection.

asturur commented 3 months ago

ok so the borders, no the controls. In that case this could make sense. I need to think carefully with nested selections, but with active selection that is never the case.

asturur commented 3 months ago

this should be ok. If it turns out it isn't i ll have to revert.

niketaOptimumnbrew commented 2 months ago

this should be ok. If it turns out it isn't i ll have to revert.

Hi @asturur I have the same issue with active selection , can you please tell me that how did you resolve that issue?

asturur commented 2 months ago

hi @niketaOptimumnbrew you should open a PR on the 5.x branch that looks like this one, do you think you can?

niketaOptimumnbrew commented 2 months ago

hi @niketaOptimumnbrew you should open a PR on the 5.x branch that looks like this one, do you think you can?

hi @asturur Thank you :) I got the answer.

asturur commented 2 months ago

i opened one few hours ago with many fixes

niketaOptimumnbrew commented 2 months ago

i opened one few hours ago with many fixes

from where can i check opened issue?

asturur commented 2 months ago

https://github.com/fabricjs/fabric.js/pulls the first one: https://github.com/fabricjs/fabric.js/pull/9965