Skip to content

Conversation

Smrtnyk
Copy link
Contributor

@Smrtnyk Smrtnyk commented Apr 19, 2025

No description provided.

Copy link

codesandbox bot commented Apr 19, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@@ -282,8 +282,8 @@ export class Control {
*/
setVisibility(
visibility: boolean,
name: string,
fabricObject: InteractiveFabricObject,
name?: string,
Copy link
Contributor Author

@Smrtnyk Smrtnyk Apr 19, 2025

Choose a reason for hiding this comment

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

not sure what these are about
they seem unused in code
also tests didn't provide them, so I just made them optional to fix type issues in tests

should these actually be removed from the function signature?

Copy link
Member

Choose a reason for hiding this comment

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

This was there to set an api. If you would override the Control class to be smarter, you could write different logic depending on what control was doing on which object.
I don't think anyone ever used it, but also there is no reason to remove it right now.

Copy link
Contributor

Build Stats

file / KB (diff) bundled minified
fabric 913.863 (0) 301.232 (0)

Copy link
Contributor

github-actions bot commented Apr 19, 2025


> fabric@6.6.2 coverage:report:ci
> nyc report --reporter=text-summary


=============================== Coverage summary ===============================
Statements   : 75.46% ( 21687/28739 )
Branches     : 60.96% ( 4113/6747 )
Functions    : 86.97% ( 1569/1804 )
Lines        : 80.08% ( 16589/20715 )
================================================================================

@asturur asturur merged commit 1e4ab40 into fabricjs:master Apr 20, 2025
20 checks passed
@Smrtnyk Smrtnyk deleted the object-interactivity branch April 20, 2025 12:45
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.

2 participants