-
Notifications
You must be signed in to change notification settings - Fork 73
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
Improve update #68
base: master
Are you sure you want to change the base?
Improve update #68
Conversation
I thought there was some case on older browsers not supporting 'input' that change would capture cases where keyup didn't (maybe right click -> paste for instance). However, at this point I think we can assume it's available: http://caniuse.com/#feat=input-event |
Looks fine to me if you are OK with #68 (comment) |
4e727bd
to
225f390
Compare
Good point: I have added a |
225f390
to
ebbd46e
Compare
this.textarea.on(inputEvent, inputHandler) | ||
this.textarea.on('change', inputHandler) | ||
this.textarea.on(inputEvent, this.update.bind(this)) | ||
if (inputEvent !== 'input') this.textarea.on('paste', inputHandler) |
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'd say either use inputHandler in both places or this.update.bind(this) in both places.
Also, 'change' seems a bit more robust vs paste - I was just bringing the paste context menu as an example - there are probably other cases where the input changes but a paste didn't happen (for instance, dragging text around).
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'd say either use inputHandler in both places or this.update.bind(this) in both places.
Apologies! Corrected in 8331b51
Also, 'change' seems a bit more robust vs paste
Yes, although it won't expand the textarea until it is blurred, which isn't a great user experience. Whilst it doesn't cover every case, handling keyup
, cut
, and paste
, should cover the most common cases on IE9.
I guess we could also handle thechange
event if input
is not supported? That way it expands immediately on keyup
, cut
and paste
, and then on change
for any other event.
To add a bit more background to this change, I am working on a messaging app, where the textarea is positioned at the bottom. As the user types and the textarea expands, the viewport's scroll position needs to be fixed to the bottom. I have something like the following:
textarea.addEventListener('expanding:update', function () {
window.scrollTo(0, document.body.scrollHeight)
})
However, if the user tries to tap the "Send" button straight after typing, expanding:update
is called on change
, the window is scrolled, which seems to prevent the touch event from submitting the form.
This only appears to happen on iOS. I have replicated this without the Expanding library here: http://jsbin.com/toxaha/edit?html,js,output
ebbd46e
to
ecd6d71
Compare
Removes the unnecessary call to update the clone textarea on
change
, since it should already have updated on theinput
event. It also exposes theupdate
method to the jQuery plugin, so that rather than accessing theExpanding
instance and callingupdate
:users can do:
as is shown in the updated demo.