-
-
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
fix(): Text broken when doing inputs #8775
Conversation
I need to write a test and verify that things like insertChars are working ro not |
Build Stats
|
this.canvas.requestRenderAll(); | ||
} | ||
return; | ||
} |
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.
moved this up since it doesn't depend from any of the let/const.
this.updateFromTextArea(); | ||
this.fire('changed'); | ||
if (this.canvas) { | ||
this.canvas.fire('text:changed', { target: this }); | ||
this.canvas.requestRenderAll(); | ||
} |
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.
deduped this
this is ready |
@ShaMan123 have a look tomorrow i want to have master fixed |
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.
This looks fine but is it enough?
What about #8711 ? It should be merged into this PR as well
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.
Is there a reason for removing the animation? I thought it is good as a showcase but it is annoying when developing
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.
that is also an internal tool, not a showcase thing.
I needed to test text, i started with my first experience with something movinng that i couldn't click, when i managed to click it i figured out it wasn't editable.
I trashed it.
There is also the issue that when running it invalidates the cache every frame and is just more work on a laptop that is rebuilding the library in the background and rebuilding the next pages.
Everyone will use the template to add something that is needed in that occasion, maybe the best default is a rect, not even a textbox
#8711 probably.
Isn't clear what is broken and what is being fixed. I will merge both |
Motivation
A recent change for removing the stateful mix has broken text input
Description
Every input is going to clear the dimension cache.
For the few cases in which we swap and
a
with anothera
in the same place, well we will pay the price of recalculation.Not a big deal.
I could have added
this._forceClearCache = true
in onInput, but sinceupdateTextarea
is the part that immediately consumes it, and is also the only place from where that function is called, i opted to haveupdateTextarea
always clear the dimension cache and set as dirty in case the initDimensions would return the exact same size ( swapping a letter with one with the same size )Added 2 tests for onInput ( a small test but better than nothing ) and one for updateFromTextarea.
Changes
Gist
In Action