Merge pull request #25528 from RudyLu/keyboard/bug985853-hold_shift

Bug 985853 - [Keyboard UX update][User Story] Hold shift to enter upper case characters.
r=timdream.

apps/keyboard/js/keyboard/active_targets_manager.js

@@ -33,6 +33,7 @@ ActiveTargetsManager.prototype.ontargetmovedin = null;
 ActiveTargetsManager.prototype.ontargetcommitted = null;
 ActiveTargetsManager.prototype.ontargetcancelled = null;
 ActiveTargetsManager.prototype.ontargetdoubletapped = null;
+ActiveTargetsManager.prototype.onnewtargetwillactivate = null;
 
 // Show accent char menu (if there is one) or do other stuff
 // after LONG_PRESS_TIMEOUT
@@ -99,11 +100,12 @@ ActiveTargetsManager.prototype._handlePressStart = function(press, id) {
     return;
   }
 
-  // All targets before the new touch need to be committed,
-  // according to UX requirement.
-  this.activeTargets.forEach(function(target, id) {
-    this._handlePressEnd(press, id);
-  }, this);
+  // Notify current targets about the new touch.
+  if (typeof this.onnewtargetwillactivate === 'function') {
+    this.activeTargets.forEach(function(target, id) {
+      this.onnewtargetwillactivate(target);
+    }, this);
+  }
 
   var target = press.target;
   this.activeTargets.set(id, target);

apps/keyboard/js/keyboard/target_handlers.js

@@ -79,6 +79,13 @@ DefaultTargetHandler.prototype.doubleTap = function() {
   this.app.console.log('DefaultTargetHandler.doubleTap()');
   this.commit();
 };
+DefaultTargetHandler.prototype.newTargetActivate = function() {
+  // According to UX requirement, the current target need to be committed when
+  // there is a new press. We will have to commit ourselves here.
+  this.commit();
+  // Ignore addition calls on commit().
+  this.ignoreCommitActions = true;
+};
 
 var NullTargetHandler = function(target, app) {
   DefaultTargetHandler.apply(this, arguments);
@@ -109,6 +116,12 @@ var CandidateSelectionTargetHandler = function(target, app) {
 CandidateSelectionTargetHandler.prototype =
   Object.create(DefaultTargetHandler.prototype);
 CandidateSelectionTargetHandler.prototype.commit = function() {
+  if (this.ignoreCommitActions) {
+    this.app.console.log(
+      'CandidateSelectionTargetHandler.commit()::return early');
+    return;
+  }
+
   this.app.candidatePanelManager.hideFullPanel();
 
   // We use the target's data instead of target.text because the
@@ -174,6 +187,8 @@ BackspaceTargetHandler.prototype.moveIn = function() {
 
 BackspaceTargetHandler.prototype.commit = function() {
   if (this.ignoreCommitActions) {
+    this.app.console.log(
+      'BackspaceTargetHandler.commit()::return early');
     return;
   }
 
@@ -198,6 +213,12 @@ var CompositeTargetHandler = function(target, app) {
 CompositeTargetHandler.prototype =
   Object.create(DefaultTargetHandler.prototype);
 CompositeTargetHandler.prototype.commit = function() {
+  if (this.ignoreCommitActions) {
+    this.app.console.log(
+      'CompositeTargetHandler.commit()::return early');
+    return;
+  }
+
   // Keys with this attribute set send more than a single character
   // Like ".com" or "2nd" or (in Catalan) "l·l".
   var compositeString = this.target.compositeKey;
@@ -215,6 +236,12 @@ var PageSwitchingTargetHandler = function(target, app) {
 PageSwitchingTargetHandler.prototype =
   Object.create(DefaultTargetHandler.prototype);
 PageSwitchingTargetHandler.prototype.commit = function() {
+  if (this.ignoreCommitActions) {
+    this.app.console.log(
+      'PageSwitchingTargetHandler.commit()::return early');
+    return;
+  }
+
   var page = this.target.targetPage;
 
   this.app.setLayoutPage(page);
@@ -229,13 +256,38 @@ PageSwitchingTargetHandler.prototype.commit = function() {
 
 var CapsLockTargetHandler = function(target, app) {
   DefaultTargetHandler.apply(this, arguments);
+
+  this.isPreviouslyUpperCase = undefined;
 };
 CapsLockTargetHandler.prototype = Object.create(DefaultTargetHandler.prototype);
-CapsLockTargetHandler.prototype.commit = function() {
+CapsLockTargetHandler.prototype.isNewTargetActivated = false;
+CapsLockTargetHandler.prototype.activate = function() {
+  this.isPreviouslyUpperCase = this.app.upperCaseStateManager.isUpperCase;
+
+  // Switch to upperCaseLocked state so all combo presses will be upper caps
   this.app.upperCaseStateManager.switchUpperCaseState({
-    isUpperCase: !this.app.upperCaseStateManager.isUpperCase,
-    isUpperCaseLocked: false
+    isUpperCaseLocked: true
   });
+
+  this.app.feedbackManager.triggerFeedback(this.target);
+  this.app.visualHighlightManager.show(this.target);
+};
+CapsLockTargetHandler.prototype.commit = function() {
+  if (this.isNewTargetActivated) {
+    // If the user have ever tap any other keys (i.e. combo keys),
+    // we should go back to lower case regardless.
+    this.app.upperCaseStateManager.switchUpperCaseState({
+      isUpperCase: false,
+      isUpperCaseLocked: false
+    });
+  } else {
+    // Depend on the previous upper case state, single tap should allow user
+    // switch between upper case and lower case.
+    this.app.upperCaseStateManager.switchUpperCaseState({
+      isUpperCase: !this.isPreviouslyUpperCase,
+      isUpperCaseLocked: false
+    });
+  }
   this.app.visualHighlightManager.hide(this.target);
 };
 CapsLockTargetHandler.prototype.doubleTap = function() {
@@ -244,6 +296,9 @@ CapsLockTargetHandler.prototype.doubleTap = function() {
   });
   this.app.visualHighlightManager.hide(this.target);
 };
+CapsLockTargetHandler.prototype.newTargetActivate = function() {
+  this.isNewTargetActivated = true;
+};
 
 var SwitchKeyboardTargetHandler = function(target, app) {
   DefaultTargetHandler.apply(this, arguments);
@@ -273,6 +328,12 @@ var ToggleCandidatePanelTargetHandler = function(target, app) {
 ToggleCandidatePanelTargetHandler.prototype =
   Object.create(DefaultTargetHandler.prototype);
 ToggleCandidatePanelTargetHandler.prototype.commit = function() {
+  if (this.ignoreCommitActions) {
+    this.app.console.log(
+      'ToggleCandidatePanelTargetHandler.commit()::return early');
+    return;
+  }
+
   this.app.candidatePanelManager.toggleFullPanel();
 
   this.app.visualHighlightManager.hide(this.target);
@@ -284,6 +345,12 @@ var DismissSuggestionsTargetHandler = function(target, app) {
 DismissSuggestionsTargetHandler.prototype =
   Object.create(DefaultTargetHandler.prototype);
 DismissSuggestionsTargetHandler.prototype.commit = function() {
+  if (this.ignoreCommitActions) {
+    this.app.console.log(
+      'DismissSuggestionsTargetHandler.commit()::return early');
+    return;
+  }
+
   var engine = this.app.inputMethodManager.currentIMEngine;
   if (typeof engine.dismissSuggestions === 'function') {
     engine.dismissSuggestions();

apps/keyboard/js/keyboard/target_handlers_manager.js

@@ -39,6 +39,8 @@ TargetHandlersManager.prototype.start = function() {
     this._callTargetAction.bind(this, 'cancel', false, true);
   activeTargetsManager.ontargetdoubletapped =
     this._callTargetAction.bind(this, 'doubleTap', false, true);
+  activeTargetsManager.onnewtargetwillactivate =
+    this._callTargetAction.bind(this, 'newTargetActivate', false, false);
   activeTargetsManager.start();
 };
 
@@ -61,12 +63,15 @@ TargetHandlersManager.prototype.stop = function() {
 // "longpress" is noticeably an optional step during the life cycle and does
 // not start or end the handler/active target, so it was not mentioned in the
 // above list.
+// "newTargetActivate" is similar to "longpress",  it is an optional step too,
+// so it was not mentioned in the above list. newTargetActivate is called on
+// the current target(s) when there is a new target is about to be activated.
 //
 // Please note that since we are using target (an abstract key object associated
 // with one DOM element) as the identifier of handlers, we do not assign new
-// handler if there are two touches on the same element. Currently that cannot
-// happen because of what done in bug 985855, however in the future that will
-// change (and these handlers needs to) to adopt bug 985853 (Combo key).
+// handler if there are two touches on the same element. If two touches happens
+// on one (maybe quite big) button, the same method on the same handler will
+// be called again with a console warning.
 TargetHandlersManager.prototype._callTargetAction = function(action,
                                                              setHandler,
                                                              deleteHandler,

apps/keyboard/style/keyboard.css

@@ -261,12 +261,12 @@ button::-moz-focus-inner {
 }
 
 /* Caps lock case */
-.keyboard-key.special-key.kbr-key-hold > .visual-wrapper > .key-element {
+.keyboard-key.special-key.kbr-key-hold:not(.highlighted) > .visual-wrapper > .key-element {
   color: #00caf2;
 }
 
 /* The underline for caps lock */
-.keyboard-key.kbr-key-hold > .visual-wrapper {
+.keyboard-key.kbr-key-hold:not(.highlighted) > .visual-wrapper {
   box-shadow: 0 0.4rem 0 #00caf2;
 }
 

apps/keyboard/test/unit/keyboard/active_targets_manager_test.js

@@ -38,6 +38,7 @@ suite('ActiveTargetsManager', function() {
     manager.ontargetcommitted = this.sinon.stub();
     manager.ontargetcancelled = this.sinon.stub();
     manager.ontargetdoubletapped = this.sinon.stub();
+    manager.onnewtargetwillactivate = this.sinon.stub();
     manager.start();
 
     assert.isTrue(window.UserPressManager.calledWithNew());
@@ -451,15 +452,13 @@ suite('ActiveTargetsManager', function() {
       setup(function() {
         userPressManagerStub.onpressstart(press1, id1);
 
-        assert.isTrue(manager.ontargetcommitted.calledWith(press0.target),
-          'Commit the first press when the second press starts.');
-        assert.isTrue(alternativesCharMenuManagerStub.hide.calledOnce);
+        assert.isTrue(manager.onnewtargetwillactivate.calledWith(press0.target),
+          'onnewtargetwillactivate called.');
 
-        assert.isTrue(
-          manager.ontargetactivated.calledWith(press1.target));
+        assert.isTrue(manager.ontargetactivated.calledWith(press1.target));
 
-        assert.equal(window.clearTimeout.callCount, 3);
-        assert.equal(window.setTimeout.callCount, 3);
+        assert.equal(window.clearTimeout.callCount, 2);
+        assert.equal(window.setTimeout.callCount, 1);
       });
 
       test('press end second press, and first press', function() {
@@ -470,10 +469,10 @@ suite('ActiveTargetsManager', function() {
         };
         userPressManagerStub.onpressend(pressEnd, id1);
 
-        assert.isTrue(alternativesCharMenuManagerStub.hide.calledTwice);
+        assert.isTrue(alternativesCharMenuManagerStub.hide.calledOnce);
         assert.isTrue(
           manager.ontargetcommitted.calledWith(press1.target));
-        assert.equal(window.clearTimeout.callCount, 4);
+        assert.equal(window.clearTimeout.callCount, 3);
 
         var pressEnd2 = {
           target: {

apps/keyboard/test/unit/keyboard/target_handlers_manager_test.js

@@ -99,6 +99,12 @@ suite('TargetHandlersManager', function() {
       assert.isTrue(handlerStub.doubleTap.calledOnce);
     });
 
+    test('New Target activated', function() {
+      activeTargetsManagerStub.onnewtargetwillactivate(target);
+
+      assert.isTrue(handlerStub.newTargetActivate.calledOnce);
+    });
+
     suite('longPress', function() {
       setup(function() {
         activeTargetsManagerStub.ontargetlongpressed(target);
@@ -129,6 +135,12 @@ suite('TargetHandlersManager', function() {
 
         assert.isTrue(handlerStub.doubleTap.calledOnce);
       });
+
+      test('New target activated', function() {
+        activeTargetsManagerStub.onnewtargetwillactivate(target);
+
+        assert.isTrue(handlerStub.newTargetActivate.calledOnce);
+      });
     });
   });
 
@@ -175,6 +187,12 @@ suite('TargetHandlersManager', function() {
       assert.isTrue(handlerStub.doubleTap.calledOnce);
     });
 
+    test('New target activated', function() {
+      activeTargetsManagerStub.onnewtargetwillactivate(target);
+
+      assert.isTrue(handlerStub.newTargetActivate.calledOnce);
+    });
+
     suite('longPress', function() {
       setup(function() {
         activeTargetsManagerStub.ontargetlongpressed(target);
@@ -205,6 +223,12 @@ suite('TargetHandlersManager', function() {
 
         assert.isTrue(handlerStub.doubleTap.calledOnce);
       });
+
+      test('New target activated', function() {
+        activeTargetsManagerStub.onnewtargetwillactivate(target);
+
+        assert.isTrue(handlerStub.newTargetActivate.calledOnce);
+      });
     });
   });
 
@@ -261,7 +285,6 @@ suite('TargetHandlersManager', function() {
       assert.isTrue(handlerStub.activate.calledOnce);
     });
 
-
     test('SpaceKeyTargetHandler', function() {
       var target = {
         keyCode: KeyEvent.DOM_VK_SPACE

apps/keyboard/test/unit/keyboard/target_handlers_test.js

@@ -131,6 +131,21 @@ suite('target handlers', function() {
         assert.isTrue(app.visualHighlightManager.hide.calledOnce);
       });
 
+      test('new target activated', function() {
+        handler.newTargetActivate();
+
+        assert.isTrue(
+          app.inputMethodManager.currentIMEngine.click.calledWith(99));
+        assert.isTrue(app.inputMethodManager.currentIMEngine.click.calledOnce);
+
+        assert.isTrue(app.visualHighlightManager.hide.calledWith(target));
+        assert.isTrue(app.visualHighlightManager.hide.calledOnce);
+
+        handler.commit();
+        // Won't commit again
+        assert.isTrue(app.inputMethodManager.currentIMEngine.click.calledOnce);
+      });
+
       suite('longPress', function() {
         setup(function() {
           handler.longPress();
@@ -722,13 +737,22 @@ suite('target handlers', function() {
     var target;
     setup(function() {
       target = {};
-
       handler = new CapsLockTargetHandler(target, app);
     });
 
     test('activate', function() {
-      assert.equal(handler.activate, DefaultTargetHandler.prototype.activate,
-        'function not overwritten');
+      handler.activate();
+
+      assert.isTrue(app.upperCaseStateManager.switchUpperCaseState
+                    .calledWith({
+        isUpperCaseLocked: true
+      }));
+
+      assert.isTrue(app.feedbackManager.triggerFeedback.calledWith(target));
+      assert.isTrue(app.feedbackManager.triggerFeedback.calledOnce);
+
+      assert.isTrue(app.visualHighlightManager.show.calledWith(target));
+      assert.isTrue(app.visualHighlightManager.show.calledOnce);
     });
 
     test('longPress', function() {
@@ -758,6 +782,20 @@ suite('target handlers', function() {
       assert.isTrue(app.visualHighlightManager.hide.calledOnce);
     });
 
+    test('combo key - activate and then with new arget activated', function() {
+      handler.newTargetActivate();
+      handler.commit();
+
+      assert.isTrue(app.upperCaseStateManager.switchUpperCaseState
+                    .calledWith({
+        isUpperCase: false,
+        isUpperCaseLocked: false
+      }));
+
+      assert.isTrue(app.visualHighlightManager.hide.calledWith(target));
+      assert.isTrue(app.visualHighlightManager.hide.calledOnce);
+    });
+
     test('cancel', function() {
       assert.equal(handler.cancel, DefaultTargetHandler.prototype.cancel,
         'function not overwritten');