AudioPlayer: Start playing a track when play icon on the track list is clicked.

AudioPlayer: Start playing a track when play icon on the track list is clicked.

Clicking a track usually changes tracks but doesn't change the play state. (play / pause)
With this CL, clicking play icon which is shown on hover will force start playing the track.

BUG=488229
TEST=manually

Committed: https://crrev.com/38df7abb0c59ef1ea11cc183ae151b6c235ff4c9
Cr-Commit-Position: refs/heads/master@{#357071}

[fukino, 2015-10-30 08:35:06]

fukino@chromium.org changed reviewers:
+ yawano@chromium.org

[fukino, 2015-10-30 08:35:06]

PTAL. Thanks!

[yawano, 2015-10-30 09:02:00]

https://codereview.chromium.org/1431523003/diff/20001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/track_list.js (right):

https://codereview.chromium.org/1431523003/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/track_list.js:170: if
(event.target.classList.contains('icon')) {
This will catch tap event of non-active and non-hover list item. i.e. When user
has tapped left side of track list (in LTR), it will start to play.

https://codereview.chromium.org/1431523003/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/track_list.js:173: this.selectTrack(track,
true);
optional nit: add comment about second parameter. e.g. this.selectTrack(track,
true /* force to play */);

[fukino, 2015-10-30 09:28:15]

Thank you!

https://codereview.chromium.org/1431523003/diff/20001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/track_list.js (right):

https://codereview.chromium.org/1431523003/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/track_list.js:170: if
(event.target.classList.contains('icon')) {
On 2015/10/30 09:02:00, yawano wrote:
> This will catch tap event of non-active and non-hover list item. i.e. When
user
> has tapped left side of track list (in LTR), it will start to play.

Good catch!
I set pointer-events: none by default.

https://codereview.chromium.org/1431523003/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/track_list.js:173: this.selectTrack(track,
true);
On 2015/10/30 09:01:59, yawano wrote:
> optional nit: add comment about second parameter. e.g. this.selectTrack(track,
> true /* force to play */);

Done.

[yawano, 2015-10-30 09:38:38]

lgtm with an optional nit. Thank you!

https://codereview.chromium.org/1431523003/diff/40001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/track_list.js (right):

https://codereview.chromium.org/1431523003/diff/40001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/track_list.js:175: this.selectTrack(track,
false /* force to play */);
optional nit: For boolean argument, we usually write comment about what this
value means. e.g. do not force to play.

e.g.
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/au...

[fukino, 2015-10-30 09:52:02]

On 2015/10/30 09:38:38, yawano wrote:
> lgtm with an optional nit. Thank you!
> 
>
https://codereview.chromium.org/1431523003/diff/40001/ui/file_manager/audio_p...
> File ui/file_manager/audio_player/elements/track_list.js (right):
> 
>
https://codereview.chromium.org/1431523003/diff/40001/ui/file_manager/audio_p...
> ui/file_manager/audio_player/elements/track_list.js:175:
this.selectTrack(track,
> false /* force to play */);
> optional nit: For boolean argument, we usually write comment about what this
> value means. e.g. do not force to play.
> 
> e.g.
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/au...

Thank you!
I'll update them later.

[fukino, 2015-10-30 09:52:05]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2015-10-30 09:52:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1431523003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1431523003/40001

[commit-bot: I haz the power, 2015-10-30 10:13:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-30 10:14:10]

Patchset 3 (id:??) landed as
https://crrev.com/38df7abb0c59ef1ea11cc183ae151b6c235ff4c9
Cr-Commit-Position: refs/heads/master@{#357071}

[fukino, 2015-11-12 08:30:35]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1435223003/ by fukino@chromium.org.

The reason for reverting is: We decided to punt redesign of audio player to
M49..