Skip to content

Commit

Permalink
CrOS: Consolidate keydown handlers in the audio player.
Browse files Browse the repository at this point in the history
Currently media keys break in tablet mode, but a 'space' keypress in
the same mode works fine. Media keys are in a separate handler that
probably lost its plumbing when shadow dom was introduced in r592709.

Merge the two key handlers into the one that works, and add a test
for 'MediaPlayPause'.

Adds a mapping for '[' and ']' to control tracks since its easier to
test and there's no prior mapping. (Cmd+Shift+][ are next/previous
tab in Chrome Mac).

Bug: 899094
Change-Id: Ie57b9b0d497b1996566ea3a1842309f0191d8bff
Reviewed-on: https://chromium-review.googlesource.com/c/1298820
Reviewed-by: Luciano Pacheco <[email protected]>
Commit-Queue: Trent Apted <[email protected]>
Cr-Commit-Position: refs/heads/master@{#602965}
  • Loading branch information
tapted authored and Commit Bot committed Oct 26, 2018
1 parent b880df2 commit be2b45d
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 58 deletions.
35 changes: 12 additions & 23 deletions ui/file_manager/audio_player/elements/audio_player.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Polymer({

listeners: {
'toggle-pause-event': 'onTogglePauseEvent_',
'next-track-event': 'onNextTrackEvent_',
'previous-track-event': 'onPreviousTrackEvent_',
'small-forward-skip-event': 'onSmallForwardSkipEvent_',
'small-backword-skip-event': 'onSmallBackwordSkipEvent_',
'big-forward-skip-event': 'onBigForwardSkipEvent_',
Expand Down Expand Up @@ -80,8 +82,6 @@ Polymer({
* element is ready.
*/
ready: function() {
this.addEventListener('keydown', this.onKeyDown_.bind(this));

this.$.audioController.addEventListener('dragging-changed',
this.onDraggingChanged_.bind(this));

Expand Down Expand Up @@ -411,27 +411,6 @@ Polymer({
}
},

/**
* Invoked when the 'keydown' event is fired.
* @param {Event} event The event object.
*/
onKeyDown_: function(event) {
switch (event.key) {
case 'MediaTrackNext':
this.onControllerNextClicked();
break;
case 'MediaPlayPause':
this.playing = !this.playing;
break;
case 'MediaTrackPrevious':
this.onControllerPreviousClicked();
break;
case 'MediaStop':
// TODO: Define "Stop" behavior.
break;
}
},

/**
* Computes volume value for audio element. (should be in [0.0, 1.0])
* @param {number} volume Volume which is set in the UI. ([0, 100])
Expand Down Expand Up @@ -480,4 +459,14 @@ Polymer({
onBigBackwordSkipEvent_: function(event) {
this.$.audioController.bigSkip(false);
},

/** @private */
onNextTrackEvent_: function(event) {
this.onControllerNextClicked();
},

/** @private */
onPreviousTrackEvent_: function(event) {
this.onControllerPreviousClicked();
},
});
12 changes: 12 additions & 0 deletions ui/file_manager/audio_player/js/audio_player.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ AudioPlayer.prototype.onKeyDown_ = function(event) {

case ' ': // Space
case 'k':
case 'MediaPlayPause':
this.player_.dispatchEvent(new Event('toggle-pause-event'));
break;
case 'ArrowUp':
Expand All @@ -403,6 +404,17 @@ AudioPlayer.prototype.onKeyDown_ = function(event) {
case 'j':
this.player_.dispatchEvent(new Event('big-backword-skip-event'));
break;
case ']':
case 'MediaTrackNext':
this.player_.dispatchEvent(new Event('next-track-event'));
break;
case '[':
case 'MediaTrackPrevious':
this.player_.dispatchEvent(new Event('previous-track-event'));
break;
case 'MediaStop':
// TODO: Define "Stop" behavior.
break;
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,41 +33,75 @@ function controlPanelQuery(query) {
testcase.togglePlayState = function() {
var openAudio = launch('local', 'downloads', [ENTRIES.beautiful]);
var appId;
return openAudio.then(function(args) {
appId = args[0];
}).then(function() {
// Audio player should start playing automatically,
return remoteCallAudioPlayer.waitForElement(
appId, 'audio-player[playing]');
}).then(function() {
// .. and the play button label should be 'Pause'.
return remoteCallAudioPlayer.waitForElement(
appId, [controlPanelQuery('#play[aria-label="Pause"]')]);
}).then(function() {
// Clicking on the play button should
return remoteCallAudioPlayer.callRemoteTestUtil(
'fakeMouseClick', appId, [controlPanelQuery('#play')]);
}).then(function() {
// ... change the audio playback state to pause,
return remoteCallAudioPlayer.waitForElement(
appId, 'audio-player:not([playing])');
}).then(function() {
// ... and the play button label should be 'Play'.
return remoteCallAudioPlayer.waitForElement(
appId, [controlPanelQuery('#play[aria-label="Play"]')]);
}).then(function() {
// Clicking on the play button again should
return remoteCallAudioPlayer.callRemoteTestUtil(
'fakeMouseClick', appId, [controlPanelQuery('#play')]);
}).then(function() {
// ... change the audio playback state to playing,
return remoteCallAudioPlayer.waitForElement(
appId, 'audio-player[playing]');
}).then(function() {
// ... and the play button label should be 'Pause'.
return remoteCallAudioPlayer.waitForElement(
appId, [controlPanelQuery('#play[aria-label="Pause"]')]);
});
return openAudio
.then(function(args) {
appId = args[0];
})
.then(function() {
// Audio player should start playing automatically,
return remoteCallAudioPlayer.waitForElement(
appId, 'audio-player[playing]');
})
.then(function() {
// .. and the play button label should be 'Pause'.
return remoteCallAudioPlayer.waitForElement(
appId, [controlPanelQuery('#play[aria-label="Pause"]')]);
})
.then(function() {

// First test a media key, before any element may have acquired focus.
return remoteCallAudioPlayer.fakeKeyDown(
appId, null, 'MediaPlayPause', false, false, false);
})
.then(function(result) {
chrome.test.assertTrue(!!result, 'fakeKeyDown failed.');
return remoteCallAudioPlayer.waitForElement(
appId, 'audio-player:not([playing])');
})
.then(function(element) {
chrome.test.assertTrue(!!element);
return remoteCallAudioPlayer.fakeKeyDown(
appId, null, 'MediaPlayPause', false, false, false);
})
.then(function(result) {
chrome.test.assertTrue(!!result, 'fakeKeyDown failed.');
return remoteCallAudioPlayer.waitForElement(
appId, 'audio-player[playing]');
})
.then(function(element) {
chrome.test.assertTrue(!!element);

// Clicking on the play button should Play.
return remoteCallAudioPlayer.callRemoteTestUtil(
'fakeMouseClick', appId, [controlPanelQuery('#play')]);
})
.then(function(result) {
chrome.test.assertTrue(!!result, 'fakeMouseClick failed.');
// ... change the audio playback state to pause,
return remoteCallAudioPlayer.waitForElement(
appId, 'audio-player:not([playing])');
})
.then(function() {
// ... and the play button label should be 'Play'.
return remoteCallAudioPlayer.waitForElement(
appId, [controlPanelQuery('#play[aria-label="Play"]')]);
})
.then(function() {
// Clicking on the play button again should
return remoteCallAudioPlayer.callRemoteTestUtil(
'fakeMouseClick', appId, [controlPanelQuery('#play')]);
})
.then(function(result) {
chrome.test.assertTrue(!!result, 'fakeMouseClick failed.');
// ... change the audio playback state to playing,
return remoteCallAudioPlayer.waitForElement(
appId, 'audio-player[playing]');
})
.then(function() {
// ... and the play button label should be 'Pause'.
return remoteCallAudioPlayer.waitForElement(
appId, [controlPanelQuery('#play[aria-label="Pause"]')]);
});
};

/**
Expand Down

0 comments on commit be2b45d

Please sign in to comment.