Display current output device icon in the floating volume row when switching output devices

ash/system/audio/volume_view.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/system/audio/volume_view.h"
 
 #include <algorithm>
 
 #include "ash/metrics/user_metrics_action.h"
 #include "ash/resources/vector_icons/vector_icons.h"
@@ skipping 113 matching lines @@
   icon_ = new VolumeButton(owner, this);
   tri_view_->AddView(TriView::Container::START, icon_);
 
   slider_ = TrayPopupUtils::CreateSlider(this);
   slider_->SetValue(CrasAudioHandler::Get()->GetOutputVolumePercent() / 100.0f);
   slider_->SetAccessibleName(
       l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_VOLUME));
   tri_view_->AddView(TriView::Container::CENTER, slider_);
 
   set_background(views::Background::CreateSolidBackground(kBackgroundColor));
-
-  if (!is_default_view_) {
-    tri_view_->SetContainerVisible(TriView::Container::END, false);
-    Update();
-    return;
-  }
-
-  more_button_ = new ButtonListenerActionableView(
-      owner_, TrayPopupInkDropStyle::INSET_BOUNDS, this);
-  TrayPopupUtils::ConfigureContainer(TriView::Container::END, more_button_);
-
-  more_button_->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
-  more_button_->SetBorder(views::CreateEmptyBorder(gfx::Insets(
-      0, kTrayPopupButtonEndMargin)));
-  tri_view_->AddView(TriView::Container::END, more_button_);
-
-  device_type_ = TrayPopupUtils::CreateMoreImageView();
-  device_type_->SetVisible(false);
-  more_button_->AddChildView(device_type_);
-
-  more_button_->AddChildView(TrayPopupUtils::CreateMoreImageView());
-  more_button_->SetAccessibleName(
-      l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_AUDIO));
   Update();
 }
 
 VolumeView::~VolumeView() {}
 
 void VolumeView::Update() {
   icon_->Update();
   slider_->UpdateState(!CrasAudioHandler::Get()->IsOutputMuted());
   UpdateDeviceTypeAndMore();
   Layout();
@@ skipping 12 matching lines @@
   slider_->SetValue(percent);
   // It is possible that the volume was (un)muted, but the actual volume level
   // did not change. In that case, setting the value of the slider won't
   // trigger an update. So explicitly trigger an update.
   Update();
   slider_->set_enable_accessibility_events(true);
 }
 
 void VolumeView::UpdateDeviceTypeAndMore() {
   CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
-  bool show_more =
-      is_default_view_ && (audio_handler->has_alternative_output() ||
-                           audio_handler->has_alternative_input());
-
-  if (!show_more)
+  // There is no point in letting the user open the audio detailed submenu if
+  // there are no alternative output devices present, so do not show the 'more'

[tdanderson, 2017/04/18 23:50:29]

nit: "if there are no alternative input or output devices present"

+  // button in the default audio row.
+  if (is_default_view_ && !audio_handler->has_alternative_output() &&

[tdanderson, 2017/04/18 23:50:29]

nit: can you run 'git cl format ash' on this? I think the conditional should be
formatted as:

if (A &&
    B &&
    C) {
}

[minch1, 2017/04/19 00:01:41]

I run 'git cl format' before upload the cl. It still looks like this. I guess it
should because the first condition is too short?

[tdanderson, 2017/04/19 15:18:09]

Ah OK, if git cl format is fine with it, then I'm fine with it too :)

+      !audio_handler->has_alternative_input()) {
+    tri_view_->SetContainerVisible(TriView::Container::END, false);
+    // TODO(tdanderson): TriView itself should trigger a relayout when the
+    // visibility of one of its containers is changed.
+    tri_view_->InvalidateLayout();
     return;
+  }
 
   const gfx::VectorIcon& device_icon = GetActiveOutputDeviceVectorIcon();
-  const bool target_visibility = !device_icon.is_empty();
-  if (target_visibility)
+  const bool device_icon_visibility = !device_icon.is_empty();
+
+  if (is_default_view_) {
+    if (!more_button_) {
+      more_button_ = new ButtonListenerActionableView(
+          owner_, TrayPopupInkDropStyle::INSET_BOUNDS, this);
+      TrayPopupUtils::ConfigureContainer(TriView::Container::END, more_button_);
+
+      more_button_->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
+      more_button_->SetBorder(
+          views::CreateEmptyBorder(gfx::Insets(0, kTrayPopupButtonEndMargin)));
+      tri_view_->AddView(TriView::Container::END, more_button_);
+
+      device_type_ = TrayPopupUtils::CreateMoreImageView();
+      device_type_->SetVisible(false);
+      more_button_->AddChildView(device_type_);
+
+      more_button_->AddChildView(TrayPopupUtils::CreateMoreImageView());
+      more_button_->SetAccessibleName(
+          l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_AUDIO));
+    }
+    tri_view_->SetContainerVisible(TriView::Container::END, true);

[tdanderson, 2017/04/18 23:50:29]

Lines 200-201 and lines 213-214 are identical and appear at the end of both 'if'
and 'else' blocks. Can you move that logic to line 216 (outside of the if-else
blocks) instead?

+    tri_view_->InvalidateLayout();
+  } else {
+    if (!device_icon_visibility) {
+      tri_view_->SetContainerVisible(TriView::Container::END, false);
+      tri_view_->InvalidateLayout();
+      return;
+    }
+    if (!device_type_) {
+      device_type_ = TrayPopupUtils::CreateMoreImageView();
+      device_type_->SetVisible(false);
+      tri_view_->AddView(TriView::Container::END, device_type_);
+    }
+    tri_view_->SetContainerVisible(TriView::Container::END, true);
+    tri_view_->InvalidateLayout();
+  }
+
+  if (device_icon_visibility)
     device_type_->SetImage(gfx::CreateVectorIcon(device_icon, kMenuIconColor));
-  if (device_type_->visible() != target_visibility) {
-    device_type_->SetVisible(target_visibility);
+  if (device_type_->visible() != device_icon_visibility) {
+    device_type_->SetVisible(device_icon_visibility);
     device_type_->InvalidateLayout();
   }
 }
 
 void VolumeView::HandleVolumeUp(int level) {
   CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
   audio_handler->SetOutputVolumePercent(level);
   if (audio_handler->IsOutputMuted() &&
       level > audio_handler->GetOutputDefaultVolumeMuteThreshold()) {
     audio_handler->SetOutputMute(false);
@@ skipping 42 matching lines @@
     if (new_volume > current_volume)
       HandleVolumeUp(new_volume);
     else
       HandleVolumeDown(new_volume);
   }
   icon_->Update();
 }
 
 }  // namespace tray
 }  // namespace ash