From 8c279093dfe64f30f8bb9ba33c1943c23a86c350 Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Fri, 27 Oct 2017 19:03:13 -0400 Subject: [PATCH] Refactor media keys and fix play/pause - Using space to play/pause was broken due to an issue with data binding. Running `$scope.$apply()` after updating the `player.pauseSong` property using `player.togglePause()` seems to have fixed this. - All of the key event processing has been moved into the JS. This means that all the key event logic is all in one spot (easier to understand IMO). Additionally, this removes the need to redefine functions for each shortcut. - Removed the now-unused `angular-ui-utils/keypress.js` package. - Fixed key events being active when editing dropdown/checkbox controls in the settings menu - Removed the custom `[Home]` shortcut since this is done natively by the browser already. - Removed the 1-6 shortcuts for the tabs. Logic being that it seems pretty rare that you would want to rapidly switch between them (you would have to use the mouse to do anything on all tabs except the subsonic one anyway), and they were already broken without anyone complaining. - Fixed tests --- app/app.js | 1 - app/common/main-controller.js | 121 ++++++++++------------------- app/common/main-controller_test.js | 53 ++++++------- app/index.html | 3 +- app/player/player-directive.js | 2 +- app/settings/settings.html | 2 - bower.json | 1 - karma.conf.js | 1 - 8 files changed, 69 insertions(+), 115 deletions(-) diff --git a/app/app.js b/app/app.js index 072da6c..54d66a0 100755 --- a/app/app.js +++ b/app/app.js @@ -6,7 +6,6 @@ angular.module('JamStash', [ 'ngRoute', 'ngSanitize', 'ngLodash', - 'ui.keypress', 'jamstash.subsonic.controller', 'jamstash.archive.controller', 'jamstash.player.controller', diff --git a/app/common/main-controller.js b/app/common/main-controller.js index 29eaa63..037805d 100644 --- a/app/common/main-controller.js +++ b/app/common/main-controller.js @@ -154,9 +154,9 @@ angular.module('JamStash') }); }; - $(document).on("click", ".message", function(){ - $(this).remove(); - }); + $(document).on("click", ".message", function(){ + $(this).remove(); + }); // Global Functions window.onbeforeunload = function () { @@ -171,39 +171,51 @@ angular.module('JamStash') $(this).fadeOut(function () { $(this).remove(); }); return false; }) - $document.keydown(function (e) { - $scope.scrollToIndex(e); + + // Shortcut processing + $(document).keydown(function (e) { + $scope.processKeyEvent(e); }); $scope.scrollToIndex = function (e) { - var source = e.target.id; - if (e.target.tagName.toUpperCase() != 'INPUT') { - var unicode = e.charCode ? e.charCode : e.keyCode; - if (globals.settings.Debug) { console.log('Keycode Triggered: ' + unicode); } - if (unicode == 49) { // 1 - $('#action_Queue').click(); - } else if (unicode == 50) { - $('#action_Library').click(); - } else if (unicode == 51) { - $('#action_Archive').click(); - } else if (unicode == 52) { - $('#action_Settings').click(); - } else if (unicode == 53) { - } else if (unicode == 54) { // 6 + $scope.processKeyEvent(e); + return true; + }; + $scope.processKeyEvent = function (e) { + if (e.isDefaultPrevented() || + e.repeat || + e.altKey || e.metaKey || e.ctrlKey || + (e.target && _.contains(['input', 'textarea', 'select'], e.target.tagName.toLowerCase()))) { + return; + } + + var key = e.key; + if (globals.settings.Debug) { console.log('Key pressed: ' + key); } + if (key == "Esc" || key == "Escape") { + $rootScope.hideQueue(); + } else if (key == " " || key == "Space") { + player.togglePause(); + } else if (key == "ArrowLeft" || key == "Left") { + player.previousTrack(); + } else if (key == "ArrowRight" || key == "Right") { + player.nextTrack(); + } else if (key == "-" || key == "_") { + persistence.saveVolume(player.turnVolumeDown()); + } else if (key == "=" || key == "+") { + persistence.saveVolume(player.turnVolumeUp()); + } else if (/^[a-z]$/i.test(key) && $('#tabLibrary').is(':visible')) { + if (/^[x-z]$/i.test(key)) { + key = 'x-z'; } - if (unicode >= 65 && unicode <= 90 && $('#tabLibrary').is(':visible')) { // a-z - var key = utils.findKeyForCode(unicode); - if (key == 'x' || key == 'y' || key == 'z') { - key = 'x-z'; - } - var el = '#' + key.toUpperCase(); - if ($(el).length > 0) { - $('#left-component').stop().scrollTo(el, 400); - } - } else if (unicode == 36 && $('#tabLibrary').is(':visible')) { // home - $('#left-component').stop().scrollTo('#MusicFolders', 400); + var el = '#' + key.toUpperCase(); + if ($(el).length > 0) { + $('#left-component').stop().scrollTo(el, 400); } } - return true; + else{ + return; + } + $scope.$apply(); + e.preventDefault(); }; $scope.scrollToIndexName = function (index) { var el = '#' + index; @@ -246,53 +258,6 @@ angular.module('JamStash') }); }; - /** - * Returns true if the target of this event is an input - * @param {jQuery event} event - * @return {Boolean} - */ - function isTargetInput (event) { - return (event && event.target.tagName === "INPUT"); - } - - /* We define player-related methods here instead of in player controller - in order to bind keypresses to and have global shortcuts. - We also check the event so we don't do anything if it's on an input */ - $scope.togglePause = function (event) { - if(!isTargetInput(event)) { - if(globals.settings.Jukebox) { - $scope.sendToJukebox('stop'); - } else { - player.togglePause(); - } - } - }; - - $scope.turnVolumeUp = function (event) { - if(!isTargetInput(event)) { - var volume = player.turnVolumeUp(); - persistence.saveVolume(volume); - } - }; - - $scope.turnVolumeDown = function (event) { - if(!isTargetInput(event)) { - var volume = player.turnVolumeDown(); - persistence.saveVolume(volume); - } - }; - - $scope.nextTrack = function (event) { - if(!isTargetInput(event)) { - player.nextTrack(); - } - }; - $scope.previousTrack = function (event) { - if(!isTargetInput(event)) { - player.previousTrack(); - } - }; - $rootScope.addToJukebox = function (id) { if (globals.settings.Debug) { console.log("LOAD JUKEBOX"); } $.ajax({ diff --git a/app/common/main-controller_test.js b/app/common/main-controller_test.js index 8780b1f..14496c2 100644 --- a/app/common/main-controller_test.js +++ b/app/common/main-controller_test.js @@ -3,7 +3,7 @@ describe("Main controller", function () { 'use strict'; var controllerParams, $controller, $q, scope, mockGlobals, player, utils, persistence, subsonic, notifications, - deferred; + deferred, mockKeypress; beforeEach(function () { mockGlobals = { settings: { @@ -18,6 +18,16 @@ describe("Main controller", function () { $provide.value('globals', mockGlobals); }); + // Mock a keypress to the application + mockKeypress = function(scope, key, target){ + scope.processKeyEvent({ + key: key, + target: target, + isDefaultPrevented: function(){}, + preventDefault: function(){} + }); + } + // Mock the player service player = jasmine.createSpyObj("player", [ "togglePause", @@ -78,10 +88,6 @@ describe("Main controller", function () { }); }); - xdescribe("toggleSetting -", function () { - - }); - describe("", function () { beforeEach(function () { $controller('AppController', controllerParams); @@ -99,24 +105,16 @@ describe("Main controller", function () { expect(scope.showQueue).toHaveBeenCalled(); }); - describe("When I toggle pause,", function () { - it("given that we're using the Jukebox mode, it sends a 'stop' command to the jukebox", function () { - mockGlobals.settings.Jukebox = true; - spyOn(scope, "sendToJukebox"); - - scope.togglePause(); - expect(scope.sendToJukebox).toHaveBeenCalledWith('stop'); - }); - - it("it toggles pause using the player service", function () { - scope.togglePause(); + describe("When I toggle pause using the keyboard shortcut,", function () { + it("it toggles pause on the player service", function () { + mockKeypress(scope, ' '); expect(player.togglePause).toHaveBeenCalled(); }); }); it("When I turn the volume up, it sets the player's volume up and saves it using the persistence service", function () { player.turnVolumeUp.and.returnValue(0.6); - scope.turnVolumeUp(); + mockKeypress(scope, '+'); expect(player.turnVolumeUp).toHaveBeenCalled(); expect(persistence.saveVolume).toHaveBeenCalledWith(0.6); @@ -124,52 +122,49 @@ describe("Main controller", function () { it("When I turn the volume down, it sets the player's volume down and saves it using the persistence service", function () { player.turnVolumeDown.and.returnValue(0.4); - scope.turnVolumeDown(); + mockKeypress(scope, '-'); expect(player.turnVolumeDown).toHaveBeenCalled(); expect(persistence.saveVolume).toHaveBeenCalledWith(0.4); }); it("When I go to the next track, it calls next track on the player", function () { - scope.nextTrack(); + mockKeypress(scope, 'ArrowRight'); expect(player.nextTrack).toHaveBeenCalled(); }); it("When I go to the previous track, it calls previous track on the player", function () { - scope.previousTrack(); + mockKeypress(scope, 'ArrowLeft'); expect(player.previousTrack).toHaveBeenCalled(); }); describe("Given that I am targeting an input,", function () { - var event; - beforeEach(function () { - event = { target: { tagName: "INPUT" } }; - }); + var target = { 'tagName': "iNPUt" } ; it("when I use a shortcut to toggle pause, it doesn't do anything", function () { - scope.togglePause(event); + mockKeypress(scope, ' ', target); expect(player.togglePause).not.toHaveBeenCalled(); }); it("when I use a shortcut to turn the volume up, it doesn't do anything", function () { - scope.turnVolumeUp(event); + mockKeypress(scope, '+', target); expect(player.turnVolumeUp).not.toHaveBeenCalled(); expect(persistence.saveVolume).not.toHaveBeenCalled(); }); it("when I use a shortcut to turn the volume down, it doesn't do anything", function () { - scope.turnVolumeDown(event); + mockKeypress(scope, '-', target); expect(player.turnVolumeDown).not.toHaveBeenCalled(); expect(persistence.saveVolume).not.toHaveBeenCalled(); }); it("when I use a shortcut to go to the next track, it doesn't do anything", function () { - scope.nextTrack(event); + mockKeypress(scope, 'RightArrow', target); expect(player.nextTrack).not.toHaveBeenCalled(); }); it("when I use a shortcut to go to the previous track, it doesn't do anything", function () { - scope.previousTrack(event); + mockKeypress(scope, 'LeftArrow', target); expect(player.previousTrack).not.toHaveBeenCalled(); }); }); diff --git a/app/index.html b/app/index.html index 7a7d9bd..20ab872 100755 --- a/app/index.html +++ b/app/index.html @@ -26,7 +26,7 @@ - +