-
-
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() BREAKING: align line to other classes for positioning #8877
Conversation
Build Stats
|
I know you don't want more changes... |
Because you can already have a polyline with 2 points. |
Ideally both polyon, polyline and line just extend Path limiting some functionality, but we need a smart way to handle the points handling afterwards. |
I m amazed that the more es6 i use, the more the bundle grow, i think is all safari fault. |
It is really strange the stats from the last prs. |
is just because every time we add a deconstruct in the arguments safari freaks out. |
you should see the template with parameters what does it do: return ['<line ', 'COMMON_PARTS', "x1=\"".concat(x1, "\" y1=\"").concat(y1, "\" x2=\"").concat(x2, "\" y2=\"").concat(y2, "\" />\n")]; |
Web dev can't just go smoothly even when things start to workout |
Can't |
this.width = Math.abs(x2 - x1); | ||
this.height = Math.abs(y2 - y1); | ||
const { left, top, width, height } = makeBoundingBoxFromPoints([ | ||
{ x: x1, y: y1 }, | ||
{ x: x2, y: y2 }, | ||
]); |
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.
redundant calc of width/height
obtain from makeBoundingBoxFromPoints
Motivation
Line has some peculiar code that could be removed in favor of standard methods we use everyewhere else.
Description
Trying to remove the custom code for the standard setPostionByOrigin and makeBoundingBoxFromPoints, leaving open the issue with changes in positioning.
This won't affect old saved data but affect lines that are created without left,top and just points.
I would seem normal that if i have a line that extends from
5,15 to 15,5
it's center is in10, 10
regardless of strokeWidth and that the left, top and originX or originY are assigned depending from the options or the defaults but not from the points themselvesToday you get different originX and originY depending on the order of your points, like if x1 or y1 are the origin, and unless you specified a different origin.
Make sense to remove those contraddictions but there are cases in which the lines aren't anymore in the previous places when initialized with the same points.
Gist
In Action