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

Better handling of text composition #3635

Merged
merged 6 commits into from
Jul 26, 2018
Merged

Better handling of text composition #3635

merged 6 commits into from
Jul 26, 2018

Conversation

nightwing
Copy link
Member

@nightwing nightwing commented Apr 6, 2018

https://rawgit.com/ajaxorg/ace/master/kitchen-sink.html

Overview

Textinput stores selection and value of textarea
After input event, sendtext checks the difference from the last value, and detects what change is needed to get from old value to the new one.
The normal case of inserting text goes through the old codepath, and there is a new handler for the case when selection needs to be modified before or after the insertion.
textarea size is set to 1px (0 on ie to hide the blinking cursor) comment in monaco was saying that there is an issue with setting it 0 on all browsers.
Font size is set to 1px on desktop, to make sure the text in textarea doesn't change the position of ime popup much, and inherit on mobile to not trigger zoom on focus
After endOperation events value of textarea is set to the current line (two lines around the cursor for multiline selections) this allows screen readers to work the same way as in textarea, and also helps with mobile keyboards that do not fire proper events
For the composition there are two modes
On chrome and firefox we set textarea value to "", make textarea visible, and add a placeholder token under it, this allows the native underline to be visible, which has information about word boundaries
on other browsers we keep textarea hidden, insert text into the document and add a marker for the underline (this is needed because on mobile all text input goes through composition events, and on ie setting value to "" from composition start doesn't work)

Testing

Manually tested windows, linux, mac with combinations of ie11, chrome, firefox, safari, for japanese and korean ime
android: chrome and firefox with gboard, samsung keyboard, swiftkey, hackers keyboard

@adamjimenez
Copy link
Contributor

This seems to fix #2038 and a bunch others.
❤️ @nightwing

@vanillajonathan
Copy link
Contributor

This works great on Android! <3

@adamjimenez
Copy link
Contributor

adamjimenez commented Apr 10, 2018

The Live autocompletion in the kitchen sink demo doesn't show on Chrome for Android. It does show on the master branch
Backspacing a new line after pressing enter takes a few presses before it deletes.
Backspacing a line seems to mess up the virtual keyboard completions.

@codecov
Copy link

codecov bot commented Jul 22, 2018

Codecov Report

Merging #3635 into master will increase coverage by 0.87%.
The diff coverage is 96.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3635      +/-   ##
==========================================
+ Coverage   67.21%   68.09%   +0.87%     
==========================================
  Files         483      485       +2     
  Lines       47927    48191     +264     
  Branches     9106     9138      +32     
==========================================
+ Hits        32214    32814     +600     
+ Misses      15713    15377     -336
Impacted Files Coverage Δ
lib/ace/lib/useragent.js 16.12% <0%> (-1.12%) ⬇️
lib/ace/mouse/mouse_handler.js 80.76% <100%> (+0.45%) ⬆️
lib/ace/keyboard/vim.js 86.74% <100%> (+2.8%) ⬆️
lib/ace/editor.js 77.44% <91.42%> (+0.43%) ⬆️
lib/ace/keyboard/textinput.js 78.57% <95.52%> (+40.38%) ⬆️
lib/ace/keyboard/vim_ace_test.js 96.25% <96.25%> (ø)
lib/ace/virtual_renderer.js 73.9% <98.21%> (+4.29%) ⬆️
lib/ace/keyboard/textinput_test.js 99.16% <99.16%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 751cbbf...1b96c9e. Read the comment docs.

@nightwing
Copy link
Member Author

Thanks for testing! The new version should fix issues with autocompletion and backspace.
And all the issues from https://github.com/ajaxorg/ace/issues?q=project:ajaxorg/ace/1

@nightwing nightwing force-pushed the better-textinput branch 8 times, most recently from 67179b4 to c51eea3 Compare July 24, 2018 11:30
@nightwing nightwing changed the title Better handling of composition Better handling of text composition Jul 25, 2018
@nightwing nightwing merged commit aca56c6 into master Jul 26, 2018
@nightwing nightwing deleted the better-textinput branch July 26, 2018 19:26
This was referenced Jul 27, 2018
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

Successfully merging this pull request may close these issues.

3 participants