AudioPlayer: Update volume controls.

AudioPlayer: Update volume controls.

This CL includes:
- Custom volume slider is replaced with paper-slider, which is dynamically shown on the top of control panel when volume button is activated.

BUG=488229
Committed: https://crrev.com/8847bc03c848c4f5dab165b9cb796a79ec69ef72
Cr-Commit-Position: refs/heads/master@{#357069}

[fukino, 2015-10-29 17:34:00]

Description was changed from

==========
AudioPlayer: Update volume controls.

BUG=
==========

to

==========
AudioPlayer: Update volume controls.

BUG=488229
==========

[fukino, 2015-10-30 07:49:47]

Description was changed from

==========
AudioPlayer: Update volume controls.

BUG=488229
==========

to

==========
AudioPlayer: Update volume controls.

This CL includes:
- Custom volume slider is replaced with paper-slider, which is dynamically shown
on the top of control panel when volume button is activated.

BUG=488229
==========

[fukino, 2015-10-30 07:50:06]

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

[fukino, 2015-10-30 07:50:07]

Awano-san, could you take a look?

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/js/audio_player.js (left):

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:342: AudioPlayer.CONTROLS_HEIGHT
= 96;
Since the control panels' height is not static anymore, I added
getControlHeight_() instead.
(Same for EXPANDED_MODE_MIN_HEIGHT)

[yawano, 2015-10-30 08:28:25]

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

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/audio_player.js:240: this.expanded =
newModel.expanded;
You need to set volumeSliderShown here.

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/control_panel.html (right):

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.html:17: <div
class="upper-controls volume-controls" hidden="[[!volumeSliderShown]]">
optional nit: how about to integrate upper-controls and lower-controls classes
into one class? e.g. control-row.

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

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:31: } else if (change.name ==
'volumeSliderShown' &&
small nit: use === instead of ==.

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:34: change.oldValue,
change.object.expanded);
change.object.volumeSliderShown?

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:394: * Invoked when the
'volumeSliderShwon' property in the model is changed.
typo: volumeSliderShwon -> volumeSliderShown

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:430: * Calculate the height of
control panel.
nit: Calculate -> Calculates

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:444: * Calculate the minium
height of the app to show the playlist in expanded mode.
nit: Calculate -> Calculates

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

Thank you!

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

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/audio_player.js:240: this.expanded =
newModel.expanded;
On 2015/10/30 08:28:25, yawano wrote:
> You need to set volumeSliderShown here.

Oops, thanks! Done.

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
File ui/file_manager/audio_player/elements/control_panel.html (right):

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/elements/control_panel.html:17: <div
class="upper-controls volume-controls" hidden="[[!volumeSliderShown]]">
On 2015/10/30 08:28:25, yawano wrote:
> optional nit: how about to integrate upper-controls and lower-controls classes
> into one class? e.g. control-row.

SGTM! Done.

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

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:31: } else if (change.name ==
'volumeSliderShown' &&
On 2015/10/30 08:28:25, yawano wrote:
> small nit: use === instead of ==.

Done.

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:34: change.oldValue,
change.object.expanded);
On 2015/10/30 08:28:25, yawano wrote:
> change.object.volumeSliderShown?

Sorry! Actually these arguments are not used.
I removed them.

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:394: * Invoked when the
'volumeSliderShwon' property in the model is changed.
On 2015/10/30 08:28:25, yawano wrote:
> typo: volumeSliderShwon -> volumeSliderShown

Done.

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:430: * Calculate the height of
control panel.
On 2015/10/30 08:28:25, yawano wrote:
> nit: Calculate -> Calculates

Done.

https://codereview.chromium.org/1425033006/diff/20001/ui/file_manager/audio_p...
ui/file_manager/audio_player/js/audio_player.js:444: * Calculate the minium
height of the app to show the playlist in expanded mode.
On 2015/10/30 08:28:25, yawano wrote:
> nit: Calculate -> Calculates

Done.

[yawano, 2015-10-30 09:13:16]

lgtm. Thank you!

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

On 2015/10/30 09:13:16, yawano wrote:
> lgtm. Thank you!

Thank you!

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

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2015-10-30 09:28:59]

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

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

Committed patchset #4 (id:60001)

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

Patchset 4 (id:??) landed as
https://crrev.com/8847bc03c848c4f5dab165b9cb796a79ec69ef72
Cr-Commit-Position: refs/heads/master@{#357069}

[fukino, 2015-11-12 08:23:54]

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

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