feat(timepicker): have up/down arrow keys control time selection #3068

joshfriend · 2014-12-08T21:11:00Z

Similar to #2316, but:

has tests/docs.
properly consumes up/down events with preventDefault.


        feat(timepicker): have up/down arrow keys control time selection

wesleycho · 2015-03-23T06:12:19Z

src/timepicker/timepicker.js

+    hoursInputEl.bind('keydown', function(e) {
+      if ( e.which === 38 ) { // up
+        $scope.$apply(function() {
+          $scope.incrementHours();


This should be rewritten as

$scope.incrementHours(); $scope.$apply();

wesleycho · 2015-03-23T06:12:31Z

src/timepicker/timepicker.js

+      }
+      else if ( e.which === 40 ) { // down
+        $scope.$apply(function() {
+          $scope.decrementHours();


Similarly here.

wesleycho · 2015-03-23T06:12:40Z

src/timepicker/timepicker.js

+        $scope.$apply(function() {
+          $scope.incrementHours();
+        });
+        e.preventDefault();


This should be at the top of the if conditional

wesleycho · 2015-03-23T06:12:48Z

src/timepicker/timepicker.js

+        $scope.$apply(function() {
+          $scope.decrementHours();
+        });
+        e.preventDefault();


wesleycho · 2015-03-23T06:13:03Z

src/timepicker/timepicker.js

+        $scope.$apply(function() {
+          $scope.incrementMinutes();
+        });
+        e.preventDefault();


Same with this if block

wesleycho · 2015-03-23T06:13:09Z

src/timepicker/timepicker.js

+        $scope.$apply(function() {
+          $scope.decrementMinutes();
+        });
+        e.preventDefault();


wesleycho · 2015-03-23T06:14:17Z

src/timepicker/timepicker.js

@@ -26,6 +27,11 @@ angular.module('ui.bootstrap.timepicker', [])
      this.setupMousewheelEvents( hoursInputEl, minutesInputEl );
    }

+    var arrowkeys = angular.isDefined($attrs.arrowkeys) ? $scope.$parent.$eval($attrs.arrowkeys) : timepickerConfig.arrowkeys;


This would be better changed to use the isolate scope in the directive.

meaning I should use $scope.$eval instead of $scope.$parent.$eval?


        refactor(timepicker): use isolated scope instead of parent scope


        refactor(timepicker): call $scope.apply after inc/dec

wesleycho · 2015-03-23T14:20:14Z

src/timepicker/timepicker.js

@@ -26,6 +27,11 @@ angular.module('ui.bootstrap.timepicker', [])
      this.setupMousewheelEvents( hoursInputEl, minutesInputEl );
    }

+    var arrowkeys = angular.isDefined($attrs.arrowkeys) ? $scope.$eval($attrs.arrowkeys) : timepickerConfig.arrowkeys;


I meant modify the directive scope to set arrowkeys: '=?'. It should be present by the time the init function is run, so you can directly pull the value from $scope.arrowkeys in that case.

Actually upon looking at the rest of the source, change this back to $scope.$parent.$eval - I would rather have consistency with the rest of the source for now.

wesleycho · 2015-03-23T14:22:29Z

This LGTM once that change is made back to use $scope.$parent.$eval.


        Revert "refactor(timepicker): use isolated scope instead of parent sc…

…ope" This reverts commit 0027763.

joshfriend · 2015-03-23T14:57:34Z

@wesleycho would you like me to rebase this branch so that 0027763 and 8eb93ce are squashed?

wesleycho · 2015-03-23T14:58:32Z

That won't be necessary, I can do the rebasing if needed.

wesleycho · 2015-03-23T15:06:23Z

Landed as 2296115, thanks!

joshfriend force-pushed the MichiganLabs:arrowkeys branch from acca308 to 8a50318 Dec 8, 2014

feat(timepicker): have up/down arrow keys control time selection

Loading status checks…

a3acc79

joshfriend force-pushed the MichiganLabs:arrowkeys branch from 8a50318 to a3acc79 Dec 9, 2014

karianna added the component: timepicker label Mar 5, 2015

wesleycho force-pushed the angular-ui:master branch from e373941 to 2c2dba6 Mar 23, 2015