 Chromium Code Reviews
 Chromium Code Reviews Issue 1491543002:
  AudioPlayer: Update layout to show the volume slider without using popup.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 1491543002:
  AudioPlayer: Update layout to show the volume slider without using popup.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: ui/file_manager/audio_player/elements/control_panel.js | 
| diff --git a/ui/file_manager/audio_player/elements/control_panel.js b/ui/file_manager/audio_player/elements/control_panel.js | 
| index 452b881d073b7746fbf0d703bed18d41029122d2..0a3063651026e6e5299274b42f376c62724bf3b4 100644 | 
| --- a/ui/file_manager/audio_player/elements/control_panel.js | 
| +++ b/ui/file_manager/audio_player/elements/control_panel.js | 
| @@ -81,7 +81,9 @@ | 
| */ | 
| volume: { | 
| type: Number, | 
| - notify: true | 
| + notify: true, | 
| + reflectToAttribute: true, | 
| + observer: 'volumeChanged_' | 
| }, | 
| /** | 
| @@ -94,16 +96,6 @@ | 
| }, | 
| /** | 
| - * Whether the volume slider is expanded or not. | 
| - */ | 
| - volumeSliderShown: { | 
| - type: Boolean, | 
| - value: false, | 
| - observer: 'volumeSliderShownChanged', | 
| - notify: true | 
| - }, | 
| - | 
| - /** | 
| * Whether the knob of time slider is being dragged. | 
| */ | 
| dragging: { | 
| @@ -126,19 +118,26 @@ | 
| * element is ready. | 
| */ | 
| ready: function() { | 
| - var onFocusoutBound = this.onVolumeControllerFocusout_.bind(this); | 
| - | 
| - this.$.volumeSlider.addEventListener('focusout', onFocusoutBound); | 
| - this.$.volumeButton.addEventListener('focusout', onFocusoutBound); | 
| - | 
| - this.$.timeSlider.addEventListener('value-change', function() { | 
| + var timeSlider = /** @type {PaperSliderElement} */ (this.$.timeSlider); | 
| + timeSlider.addEventListener('value-change', function() { | 
| 
yawano
2015/12/01 07:51:31
value-change -> change?
 
fukino
2015/12/02 04:52:03
Yes, it should be 'change'. Done.
 | 
| if (this.dragging) | 
| this.dragging = false; | 
| }.bind(this)); | 
| - this.$.timeSlider.addEventListener('immediate-value-change', function() { | 
| + timeSlider.addEventListener('immediate-value-change', function() { | 
| if (!this.dragging) | 
| this.dragging = true; | 
| }.bind(this)); | 
| + | 
| + // Update volume on user inputs for volume slider. | 
| + // During a drag operation, the volume should be updated immediately. | 
| + var volumeSlider = | 
| + /** @type {PaperSliderElement} */ (this.$.volumeSlider); | 
| + volumeSlider.addEventListener('change', function() { | 
| + this.volume = volumeSlider.value; | 
| + }.bind(this)); | 
| + volumeSlider.addEventListener('immediate-value-change', function() { | 
| + this.volume = volumeSlider.immediateValue; | 
| + }.bind(this)); | 
| }, | 
| /** | 
| @@ -163,40 +162,14 @@ | 
| }, | 
| /** | 
| - * Invoked when the property 'volumeSliderShown' changes. | 
| - * @param {boolean} shown | 
| + * Invoked when the volume button is clicked. | 
| */ | 
| - volumeSliderShownChanged: function(shown) { | 
| - this.showVolumeController_(shown); | 
| - }, | 
| - | 
| - /** | 
| - * Invoked when the focus goes out of the volume elements. | 
| - * @param {!UIEvent} event The focusout event. | 
| - * @private | 
| - */ | 
| - onVolumeControllerFocusout_: function(event) { | 
| - if (this.volumeSliderShown) { | 
| - // If the focus goes out of the volume, hide the volume control. | 
| - if (!event.relatedTarget || | 
| - (!this.$.volumeButton.contains(event.relatedTarget) && | 
| - !this.$.volumeSlider.contains(event.relatedTarget))) { | 
| - this.volumeSliderShown = false; | 
| - } | 
| - } | 
| - }, | 
| - | 
| - /** | 
| - * Shows/hides the volume controller. | 
| - * @param {boolean} show True to show the controller, false to hide. | 
| - * @private | 
| - */ | 
| - showVolumeController_: function(show) { | 
| - if (show) { | 
| - matchBottomLine(this.$.volumeContainer, this.$.volumeButton); | 
| - this.$.volumeContainer.style.visibility = 'visible'; | 
| + volumeClick: function() { | 
| + if (this.volume !== 0) { | 
| + this.savedVolume_ = this.volume; | 
| + this.volume = 0; | 
| } else { | 
| - this.$.volumeContainer.style.visibility = 'hidden'; | 
| + this.volume = this.savedVolume_ || 50; | 
| } | 
| }, | 
| @@ -232,6 +205,21 @@ | 
| }, | 
| /** | 
| + * Invoked when the volume property is changed. | 
| + * @param {number} volume | 
| + * @private | 
| + */ | 
| + volumeChanged_: function(volume) { | 
| + if (!this.$.volumeSlider.dragging) | 
| + this.$.volumeSlider.value = volume; | 
| + | 
| + if (this.ariaLabels) { | 
| + this.$.volumeButton.setAttribute('aria-label', | 
| 
yawano
2015/12/01 07:51:31
This might be out of the scope of this CL. If you
 
fukino
2015/12/02 04:52:02
I'm not a big fun of tooltips when the buttons use
 
yawano
2015/12/02 06:11:43
Acknowledged.
 | 
| + volume !== 0 ? this.ariaLabels.mute : this.ariaLabels.unmute); | 
| + } | 
| + }, | 
| + | 
| + /** | 
| * Invoked when the ariaLabels property is changed. | 
| * @param {Object} ariaLabels | 
| * @private | 
| @@ -249,6 +237,9 @@ | 
| this.$.volumeButton.setAttribute('aria-label', ariaLabels.volume); | 
| this.$.playList.setAttribute('aria-label', ariaLabels.playList); | 
| this.$.timeSlider.setAttribute('aria-label', ariaLabels.seekSlider); | 
| + this.$.volumeButton.setAttribute('aria-label', | 
| + this.volume !== 0 ? ariaLabels.mute : ariaLabels.unmute); | 
| + this.$.volumeSlider.setAttribute('aria-label', ariaLabels.volumeSlider); | 
| } | 
| }); | 
| })(); // Anonymous closure |