fix(forms): make composition event buffering configurable #15256

kara · 2017-03-17T20:30:03Z

This commit fixes a regression where ngModel no longer syncs letter by letter on Android devices, and instead syncs at the end of every word. This broke when we introduced buffering of IME events so IMEs like Pinyin keyboards or Katakana keyboards wouldn't display composition strings. Unfortunately, iOS devices and Android devices have opposite event behavior. Whereas iOS devices fire composition events for IME keyboards only, Android fires composition events for Latin-language keyboards. For this reason, languages like English don't work as expected on Android if we always buffer. So to support both platforms, composition string buffering will only be turned on by default for non-Android devices.

However, we have also added a COMPOSITION_BUFFER_MODE token to make this configurable by the application, because in some cases, apps might might still want to receive intermediate values. For example, some inputs begin searching behavior based on Latin letters before a character selection is made.

As a provider, this is fairly flexible. If you want to turn composition buffering off, simply provide the token at the top level:

providers: [
   {provide: COMPOSITION_BUFFER_MODE, useValue: false}
]

Or, if you want to change the mode based on locale or platform, you can use a factory:

import {shouldUseBuffering} from 'my/lib';

....
providers: [
   {provide: COMPOSITION_BUFFER_MODE, useFactory: shouldUseBuffering}
]

Closes #15079.

robwormald · 2017-03-17T21:42:59Z

packages/forms/src/directives/default_value_accessor.ts

-  host: {'(input)': 'onChange($event.target.value)', '(blur)': 'onTouched()'},
+  // selector: '[ngModel],[formControl],[formControlName]',
+  host: {
+    '(input)': '_handleInput($event.target.value)',


does the compositionupdate event come into this at all? Does it make sense to listen to that instead of input ?

Talked in person about this

robwormald · 2017-03-17T21:46:06Z

LGTM once tests green

tinayuangao

LGTM

IgorMinar · 2017-03-20T23:10:53Z

tools/public_api_guard/forms/forms.d.ts

@@ -140,7 +143,10 @@ export interface ControlValueAccessor {
 export declare class DefaultValueAccessor implements ControlValueAccessor {
    onChange: (_: any) => void;
    onTouched: () => void;
-    constructor(_renderer: Renderer, _elementRef: ElementRef);
+    constructor(_renderer: Renderer, _elementRef: ElementRef, _compositionMode: boolean);
+    _compositionEnd(value: any): void;


it's a bit weird that you decided to use _ prefix just for these methods. they are all "private" no?

Talked in person about this

they must be public because they are used in event bindings. But will it work with @internal? just curious.

IgorMinar · 2017-03-20T23:16:47Z

packages/forms/test/template_integration_spec.ts

@@ -1166,6 +1133,67 @@ export function main() {

    });

+    describe('IME events', () => {


based on how these tests are written, it is not clear what's the default and that the default works on both android and ios.

is it feasible to change the tests to test this?

This commit fixes a regression where `ngModel` no longer syncs letter by letter on Android devices, and instead syncs at the end of every word. This broke when we introduced buffering of IME events so IMEs like Pinyin keyboards or Katakana keyboards wouldn't display composition strings. Unfortunately, iOS devices and Android devices have opposite event behavior. Whereas iOS devices fire composition events for IME keyboards only, Android fires composition events for Latin-language keyboards. For this reason, languages like English don't work as expected on Android if we always buffer. So to support both platforms, composition string buffering will only be turned on by default for non-Android devices. However, we have also added a `COMPOSITION_BUFFER_MODE` token to make this configurable by the application. In some cases, apps might might still want to receive intermediate values. For example, some inputs begin searching based on Latin letters before a character selection is made. As a provider, this is fairly flexible. If you want to turn composition buffering off, simply provide the token at the top level: ```ts providers: [ {provide: COMPOSITION_BUFFER_MODE, useValue: false} ] ``` Or, if you want to change the mode based on locale or platform, you can use a factory: ```ts import {shouldUseBuffering} from 'my/lib'; .... providers: [ {provide: COMPOSITION_BUFFER_MODE, useFactory: shouldUseBuffering} ] ``` Closes angular#15079.

caused by angular/angular#15256 fixes #831

) This commit fixes a regression where `ngModel` no longer syncs letter by letter on Android devices, and instead syncs at the end of every word. This broke when we introduced buffering of IME events so IMEs like Pinyin keyboards or Katakana keyboards wouldn't display composition strings. Unfortunately, iOS devices and Android devices have opposite event behavior. Whereas iOS devices fire composition events for IME keyboards only, Android fires composition events for Latin-language keyboards. For this reason, languages like English don't work as expected on Android if we always buffer. So to support both platforms, composition string buffering will only be turned on by default for non-Android devices. However, we have also added a `COMPOSITION_BUFFER_MODE` token to make this configurable by the application. In some cases, apps might might still want to receive intermediate values. For example, some inputs begin searching based on Latin letters before a character selection is made. As a provider, this is fairly flexible. If you want to turn composition buffering off, simply provide the token at the top level: ```ts providers: [ {provide: COMPOSITION_BUFFER_MODE, useValue: false} ] ``` Or, if you want to change the mode based on locale or platform, you can use a factory: ```ts import {shouldUseBuffering} from 'my/lib'; .... providers: [ {provide: COMPOSITION_BUFFER_MODE, useFactory: shouldUseBuffering} ] ``` Closes angular#15079. PR Close angular#15256

angular-automatic-lock-bot · 2019-09-11T03:21:13Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

kara added the state: WIP label Mar 17, 2017

googlebot added the cla: yes label Mar 17, 2017

robwormald reviewed Mar 17, 2017

View reviewed changes

kara force-pushed the fix-mobile branch 2 times, most recently from ffee582 to 971a5b5 Compare March 17, 2017 21:53

kara added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Mar 17, 2017

kara requested review from mhevery and IgorMinar March 17, 2017 22:02

kara assigned mhevery Mar 17, 2017

kara requested a review from tinayuangao March 17, 2017 22:56

kara assigned tinayuangao Mar 17, 2017

kara force-pushed the fix-mobile branch from 971a5b5 to 53cf3c1 Compare March 17, 2017 23:03

tinayuangao reviewed Mar 20, 2017

View reviewed changes

kara force-pushed the fix-mobile branch 3 times, most recently from 5169b41 to b7ce422 Compare March 20, 2017 21:20

IgorMinar reviewed Mar 20, 2017

View reviewed changes

kara force-pushed the fix-mobile branch from b7ce422 to 304ae50 Compare March 20, 2017 23:24

kara added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Mar 20, 2017

kara force-pushed the fix-mobile branch 3 times, most recently from 46d8c3a to acae946 Compare March 21, 2017 00:20

kara force-pushed the fix-mobile branch from acae946 to 0e25f5c Compare March 21, 2017 00:39

kara removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Mar 21, 2017

IgorMinar approved these changes Mar 21, 2017

View reviewed changes

IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 21, 2017

mhevery added the pr_action: queued label Mar 21, 2017

mhevery closed this in 5efc860 Mar 21, 2017

not-for-me mentioned this pull request May 5, 2017

multi-byte composition failed in Internet Explorer #15947

Closed

gms1 mentioned this pull request Jun 8, 2017

'NativeScriptDomAdapter.getUserAgent' implementation request NativeScript/nativescript-angular#831

Closed

sis0k0 added a commit to NativeScript/nativescript-angular that referenced this pull request Jun 8, 2017

fix(dom-adapter): add fake implementation for getUserAgent

bf5bd2a

caused by angular/angular#15256 fixes #831

sis0k0 mentioned this pull request Jun 8, 2017

fix(dom-adapter): add fake implementation for getUserAgent NativeScript/nativescript-angular#835

Merged

sis0k0 added a commit to NativeScript/nativescript-angular that referenced this pull request Jun 9, 2017

fix(dom-adapter): add fake implementation for getUserAgent (#835)

743131c

caused by angular/angular#15256 fixes #831

angular-automatic-lock-bot bot unassigned tinayuangao Sep 11, 2019

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(forms): make composition event buffering configurable #15256

fix(forms): make composition event buffering configurable #15256

Uh oh!

kara commented Mar 17, 2017 •

edited

Loading

Uh oh!

robwormald Mar 17, 2017

Uh oh!

kara Mar 17, 2017

Uh oh!

robwormald commented Mar 17, 2017

Uh oh!

tinayuangao left a comment

Uh oh!

IgorMinar Mar 20, 2017

Uh oh!

kara Mar 20, 2017

Uh oh!

DzmitryShylovich Mar 20, 2017

Uh oh!

IgorMinar Mar 20, 2017

Uh oh!

angular-automatic-lock-bot bot commented Sep 11, 2019

Uh oh!

Uh oh!

		@@ -1166,6 +1133,67 @@ export function main() {

		});

		describe('IME events', () => {

fix(forms): make composition event buffering configurable #15256

fix(forms): make composition event buffering configurable #15256

Uh oh!

Conversation

kara commented Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robwormald Mar 17, 2017

Choose a reason for hiding this comment

Uh oh!

kara Mar 17, 2017

Choose a reason for hiding this comment

Uh oh!

robwormald commented Mar 17, 2017

Uh oh!

tinayuangao left a comment

Choose a reason for hiding this comment

Uh oh!

IgorMinar Mar 20, 2017

Choose a reason for hiding this comment

Uh oh!

kara Mar 20, 2017

Choose a reason for hiding this comment

Uh oh!

DzmitryShylovich Mar 20, 2017

Choose a reason for hiding this comment

Uh oh!

IgorMinar Mar 20, 2017

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot bot commented Sep 11, 2019

Uh oh!

Uh oh!

kara commented Mar 17, 2017 •

edited

Loading