-
-
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
refactor(): getActiveControl
return value
#9515
Changes from 9 commits
44ce093
4c2b3d5
30fd773
8778ca7
763daa6
aa8dbac
82f86c6
3dee2e3
ae4b13a
5191ec5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. migrated test from |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ export const NOT_ALLOWED_CURSOR = 'not-allowed'; | |
*/ | ||
export const getActionFromCorner = ( | ||
alreadySelected: boolean, | ||
corner: string, | ||
corner: string | undefined, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not using the '?' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its usually better to init all props even if undefined, rather than having some missing and some not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't remember There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh because it can't be optional because it is not the last arg There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh i thought it was an object. |
||
e: TPointerEvent, | ||
target: FabricObject | ||
) => { | ||
|
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.
those changes here are applied by the minifier later on, but imho they take away on readibility. In the default safe lint or prettier profile are not considered, so i won't hold anything on it since may fall in the opinion realm