-
-
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(TS): migrate Pattern
#8255
Conversation
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.
Standard es6/TS, except the commented line which is a cleanup for an old PR
}; | ||
})(typeof exports !== 'undefined' ? exports : window); | ||
toSVG({ width, height }: TSize) { | ||
const patternSource = this.source, |
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.
This reverts commit 03d94c3.
* @returns true if {@link source} is a <canvas> element | ||
*/ | ||
isCanvasSource(): this is TCanvasSource { | ||
return typeof this.source === 'object' && this.source.toDataURL; |
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.
Is this an assertion for a canvas element? Or for a fabric Object? Or both?
const img = await loadImage(source, { | ||
...options, | ||
crossOrigin: serialized.crossOrigin | ||
}) | ||
return new Pattern({ ...serialized, source: img }); |
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.
everywhere else we use the promise syntax, let's leave that one
it could have them :) but patterns are usually small. |
This is fine for where we are at. Types makes it hard at dev time, classes makes it hard at runtime. Now Patterns and Gradients are usually not really where people add businnes logic ( names or special props or special behavior ) so those are fine to be standard classes, with or without the default values issue |
Migrate Pattern to TS
I thought it had default values in export, that's why I left it but it doesn't so I migrated it.