Skip to content
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

Textbox Scaling #2358

Closed
inssein opened this issue Jul 23, 2015 · 9 comments
Closed

Textbox Scaling #2358

inssein opened this issue Jul 23, 2015 · 9 comments

Comments

@inssein
Copy link
Contributor

inssein commented Jul 23, 2015

When I ported over @jafferhaider's code, there was some functionality I didn't quite understand, and I am just now getting around to it.

Something I ran into today is https://github.com/kangax/fabric.js/blob/master/src/shapes/textbox.class.js#L362. Not sure if I quite understand this, but it seems backwards to me. The point of scale is to multiply how large the displayed object is, and this is essentially reversing it.

If the object is scaled 5 times, and the fontSize is 10px, it should be the user interface's job convert that to 50px, and not the other way around.

At it's current state, any Textbox with a scale is broken. In my own application, Textbox's have scale so they can be shown larger. (Not the same thing as changing the fontSize, as kerning is different).

I will be submitting a PR that will be removing this functionality, unless there are arguments otherwise.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@jafferhaider
Copy link
Contributor

Yep, you've understood this code correctly. This behavior is intentional. My motivation for doing this was to make this widget behave the same as conventional text boxes. The normal behavior for text boxes is to allow the user to change its dimensions (by dragging on handles), which dictates text wrapping. But if instead of resizing, dragging handles scales the text box, then how will a user ever be able to change the dimensions of a text box? Overloading a handle with two different behaviors or having two types of handles will result in usability issues.

Also, there is no clear formula between scale and font size. Differences in font size varies from font to font. We could probably approximate the value, but there will be discrepancies when a user changes the font of a scaled text box.

Again, I'd urge you to think very carefully about usability implications of what you're suggesting. I'm open about adding different behavior to the Textbox if there's a real need for it ... as long as this behavior is preserved via a flag or mode variable.

@inssein
Copy link
Contributor Author

inssein commented Jul 24, 2015

Ah, I see. This was initially confusing for me because in my application, I actually override all scale transformations to be transformations of width and height, and I deal with images slightly differently.

I will think about this, and hopefully get some input from everyone else. For now, my own application actually extends Textbox, and I override setOnGroup in there to do nothing, so no urgent need to fix this.

@asturur
Copy link
Member

asturur commented Jul 24, 2015

sorry guys, does textbox scales normally?

not by corners but by code, mytextbox.scaleX = 2 ?

can this be done?

@jafferhaider
Copy link
Contributor

@asturur Yes, the scaleX and scaleY properties work.

@kangax
Copy link
Member

kangax commented Jul 25, 2015

Can someone please add textbox to our kitchensink, for easier testing?

@asturur
Copy link
Member

asturur commented Jul 25, 2015

i ll submit to fabricjs.com the mod to have also a tab to load and save
json.
textbox, we need also some skew buttons and a transformMatrix /
viewportTransform buttons.
Il 25/lug/2015 07:49 PM, "Juriy Zaytsev" notifications@github.com ha
scritto:

Can someone please add textbox to our kitchensink, for easier testing?


Reply to this email directly or view it on GitHub
#2358 (comment).

@kangax
Copy link
Member

kangax commented Jul 25, 2015

Sure! I added a separate demo for transforms but it's definitely good to
have it in kitchensink coz then we can try it with text and other objects
(with other properties).

On Sat, Jul 25, 2015 at 2:00 PM, Andrea Bogazzi notifications@github.com
wrote:

i ll submit to fabricjs.com the mod to have also a tab to load and save
json.
textbox, we need also some skew buttons and a transformMatrix /
viewportTransform buttons.
Il 25/lug/2015 07:49 PM, "Juriy Zaytsev" notifications@github.com ha
scritto:

Can someone please add textbox to our kitchensink, for easier testing?


Reply to this email directly or view it on GitHub
<#2358 (comment)
.


Reply to this email directly or view it on GitHub
#2358 (comment).

@kangax
Copy link
Member

kangax commented Aug 31, 2015

Anything else to do here?

@jafferhaider
Copy link
Contributor

Doesn't seem like it. As @inssein mentions, he's extended Textbox for this functionality which is specific to his app.

@kangax kangax closed this as completed Sep 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants