-
-
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): Polyline/Polygon #8417
Conversation
Build Stats
*inaccurate, see link |
UT fail in deepEquals F it
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.
READY
* @todo set default to true and remove flag and related logic | ||
*/ | ||
exactBoundingBox: false, | ||
private initialized: true | undefined; |
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 typed it this way since setting it to false by default is pointless because the value will be assigned after super is called and by then _set
has been called as well. The whole point of the flag is to prevent that call.
So it is undefined while calling super and true once super is ready.
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.
yeah we have to rework this. there is something similar also in text and just can't be. There must be a way to make the code straight.
src/shapes/polyline.class.ts
Outdated
|
||
export const polylineDefaultValues: Partial<TClassProperties<Polyline>> = { | ||
type: 'polyline', | ||
points: [], |
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 null anymore, maybe should be
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.
or no default value actually
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.
instead of returning point from `setDimensions`
* @see: http://www.w3.org/TR/SVG/shapes.html#PolylineElement | ||
*/ | ||
fabric.Polyline.ATTRIBUTE_NAMES = fabric.SHARED_ATTRIBUTES.concat(); | ||
static ATTRIBUTE_NAMES = [...SHARED_ATTRIBUTES]; |
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.
static ATTRIBUTE_NAMES = [...SHARED_ATTRIBUTES]; |
not needed since it is inherited from super class
Co-authored-by: Andrea Bogazzi <andreabogazzi79@gmail.com>
Motivation
Description
Changes
Polyline#isOpen(): true
as inisOpenPath
that Polygon overrides tofalse
, removing duplicate logicPoint#type
, an annoying thing that makes tests fail when converting IPoint to Point, useless and dangerous if spreading a point with an object since{ ...point, ...objectWithType }
is not deepEqual{ ...objectWithType, ...point }
Gist
In Action