-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
refactor(): create Filler base class for Pattern/Gradient #8947
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
Conversation
commit 5ae70f4 Author: ShaMan123 <[email protected]> Date: Mon May 22 11:20:33 2023 +0900 Revert "Update changelog_update.yml" This reverts commit da3ce0d. commit 951d210 Author: ShaMan123 <[email protected]> Date: Mon May 22 11:16:01 2023 +0900 Update CHANGELOG.md commit c502b49 Merge: da3ce0d f490cdf Author: ShaMan123 <[email protected]> Date: Mon May 22 11:15:12 2023 +0900 Merge branch 'rm-filler-type-assertions' of https://github.com/fabricjs/fabric.js into rm-filler-type-assertions commit da3ce0d Author: ShaMan123 <[email protected]> Date: Mon May 22 11:14:53 2023 +0900 Update changelog_update.yml commit f490cdf Author: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Date: Mon May 22 02:11:24 2023 +0000 update CHANGELOG.md commit c94bf70 Author: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Date: Mon May 22 02:11:05 2023 +0000 update CHANGELOG.md commit ce7939c Author: ShaMan123 <[email protected]> Date: Mon May 22 11:02:58 2023 +0900 mv isFiller/TFiller commit b9a9bcd Author: ShaMan123 <[email protected]> Date: Mon May 22 10:53:58 2023 +0900 type assertions commit cf40c1a Author: ShaMan123 <[email protected]> Date: Mon May 22 10:17:12 2023 +0900 init
Build Stats
|
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.
Reviewing:
src/fillers/Filler.ts
src/gradient/Gradient.ts
,src/Pattern/Pattern.ts
src/util/typeAssertions.ts
- the rest
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.
Not a lot of changes, most files are import changes
? matrixToSVG(invertTransform(this.viewportTransform)) | ||
: ''; | ||
const { width = finalWidth, height = finalHeight } = | ||
filler instanceof Pattern |
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.
change, importing Pattern
* @return {object} | ||
*/ | ||
toObject(propertiesToInclude?: (keyof this | string)[]) { | ||
toObject<T extends keyof this>(propertiesToInclude?: T[]) { |
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 am not attached to this at all
|
Also I wanted to move pattern and gradient under filler but didn't to keep this PR clean |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs 😔. Thank you for your contributions. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs 😔. Thank you for your contributions. |
Motivation
instance of Filler
as part of fix():typeAssertions
not respecting subclasses #8931Pattern
Gradient
transform #8263Description
Changes
isFiller
,TFiller
underfiller
=> caused a lot of import changesisPattern
in favor of instance of used only by canvas. We can add a dummy super class if you prefer to doinstanceof
checks against it but importing Pattern is ok, not cycles.Gist
In Action