-
-
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
[Feature]: Updating width and height of an ellipse should update rx and ry and vice versa #10179
Comments
You can also just make some controls that update rx and ry instead of width and height. |
I see your point @asturur, but if those controls were designed specifically for textbox, then why not designing specific control behavior for other shapes as well? I know that we should design the library to be generic, but since we already implemented different shapes, we should tweak the default controls to be specific to each shape. |
Also I'm not sure how to change the default behavior of the controls without having to redefine them all. My initial approach was to listen for the |
I also have this confusion. I think the code should be as simple as possible. I hope the ellipse only has width and height, and that rx and ry are read-only. Wouldn't the following be more convenient? class Ellipse2 extends FabricObject {
get rx() {
return this.width / 2;
}
get ry() {
return this.height / 2;
}
// If backward compatibility is not a concern, the two methods below can be removed.
// If users want to set rx and ry, let them set the width and height instead.
// set rx(n: number) {
// this.width = n * 2;
// }
// set ry(n: number) {
// this.height = n * 2;
// }
} |
i m against the 'viceversa' point. width and height as properties you can change works good for images and rect and nothing else. On text it make sense only to change width because height is a consequence on polygon, paths and circle and ellipse are consequences of other properties r, rx, ry, points and commands. In my opinion because of polygons and paths and text we have to move away from the width and height concept for most shapes. They need to exist internally as private properties and developers have to interact with the rx, r, ry, points and commands when trying to change a shape, while rect and image can continue to support width and height getters. This is a major change tho For what regards custom controls for ellipse and circle i can make a demo, it would help many people understand how to work with those. I ll do the one for ellipse that maybe is the most clarifying one |
CheckList
Description
Hi all,
The way ellipses are implemented for now is: if you update
rx
orry
thewidth
andheight
get updated accordingly, but not the other way around (if you update thewidth
orheight
therx
andry
don't get updated) which is more intuitive to actually do in my opinion.The decision for this implementation was made here: #1062
This is a real problem, because when you try to resize an ellipse using the UI controls, it actually changes the
width
andheight
of the ellipse, which has no effect on the shape itself, but only updates the "container" of the shape.It's easy enough to fix this in my own project, but I feel like everybody will have to do the same, since no one wants to resize a shape using the controls and having only the container changing.
The same behavior is implemented in circles, so we might fix that as well.
This is what you get when you try to resize an ellipse using the UI controls:
Current State
As a workaround for now, we can update these properties accordingly by listening to the relevant events.
Additional Context
No response
The text was updated successfully, but these errors were encountered: