-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
chore(): use deconstruction and constants in place of strings to save some bytes of code #9593
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Build Stats
|
…js/fabric.js into investigate-layoutmanager-binding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In an ideal condition I would ask this to be merged once our queue is empty to avoid annoying merge conflicts with open PRs because it is a lot of diff ink
But we are not there yet so it is what it is
src/canvas/canvas_gestures.mixin.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the fate of this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at some point we need to find a solution for gestures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave some thought.
We can introduce a control that fits the entire object for a 2 finger gesture that scales and rotates the object.
The math sounds straight forward.
Measuring the diff of the vector created between the 2 touches.
Then the dev can do for 3 fingers whatever based on the 2 finger control.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would then argue that a mobile/touch app will not want the default controls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then touch become a first class citizen in fabric
src/constants.ts
Outdated
export const ROTATE = 'rotate'; | ||
export const SKEWING = 'skewing'; | ||
export const RESIZING = 'resizing'; | ||
export const MODIFYPOLY = 'modifypoly'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we rename this?
src/controls/scaleSkew.ts
Outdated
return isAlternative ? 'skewX' : 'scaleY'; | ||
return isAlternative ? 'skewX' : SCALE_Y; | ||
} | ||
if (control.y === 0) { | ||
// then is scaleY or skewX | ||
return isAlternative ? 'skewY' : 'scaleX'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skewX/Y?
this can wait a lot is not important, i more wanted to see how much margin there was for improvements with strings |
Description
This is no functional changes a tentative to recover some bundle size
In Action