-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Ios keyboard #3310
Ios keyboard #3310
Conversation
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 see you reviewed and refactored this already; nicely done. Only minor comments, please have a look and merge.
} | ||
event.addListener(text, "compositionend", onCompositionEnd); | ||
|
||
this.getElement = function() { |
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.
For this entire block: could you reorder the code a bit so we don't interleave function and member declarations and initialization code too much?
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'll do this when refactoring textinput.js, i want to keep these files in sync to be able to easily merge them in the future.
}, 100); | ||
}); | ||
|
||
// IOS doesn't fire events for arrow keys, but this unique hack changes everything! |
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 sounds like clickbait :D
@nightwing also please make sure to update #3172 if this is merged |
@lennartcl this is the rebased version of https://github.com/ajaxorg/ace/pull/3172、i've kept ios textinput in a separate file for now, since textinput already has too many code paths