Update shelf spacing in Chrome OS according to the MD specs

ash/system/status_area_widget_delegate.cc

 // Copyright (c) 2012 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/status_area_widget_delegate.h"
 
 #include "ash/ash_export.h"
 #include "ash/ash_switches.h"
+#include "ash/common/material_design/material_design_controller.h"
+#include "ash/common/shelf/shelf_constants.h"
 #include "ash/common/shelf/wm_shelf_util.h"
 #include "ash/common/shell_window_ids.h"
 #include "ash/common/system/tray/tray_constants.h"
 #include "ash/focus_cycler.h"
 #include "ash/shelf/shelf_util.h"
 #include "ash/shell.h"
+#include "ash/system/tray/tray_background_view.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/animation/tween.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image.h"
 #include "ui/views/accessible_pane_view.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/widget/widget.h"
@@ skipping 67 matching lines @@
   // We don't want mouse clicks to activate us, but we need to allow
   // activation when the user is using the keyboard (FocusCycler).
   const FocusCycler* focus_cycler = focus_cycler_for_testing_ ?
       focus_cycler_for_testing_ : Shell::GetInstance()->focus_cycler();
   return focus_cycler->widget_activating() == GetWidget();
 }
 
 void StatusAreaWidgetDelegate::DeleteDelegate() {
 }
 
-void StatusAreaWidgetDelegate::AddTray(views::View* tray) {
+void StatusAreaWidgetDelegate::AddTray(TrayBackgroundView* tray) {
   SetLayoutManager(NULL);  // Reset layout manager before adding a child.
   AddChildView(tray);
   // Set the layout manager with the new list of children.
   UpdateLayout();
 }
 
 void StatusAreaWidgetDelegate::UpdateLayout() {
   // Use a grid layout so that the trays can be centered in each cell, and
   // so that the widget gets laid out correctly when tray sizes change.
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
 
+  // Update tray border based on layout.
+  bool is_child_on_edge = true;
+  for (int c = 0; c < child_count(); ++c) {
+    views::View* child = child_at(c);
+    if (!child->visible())
+      continue;
+    if (is_child_on_edge) {
+      SetBorderOnEdgeTray(child);
+    } else {
+      SetBorderOnNonEdgeTray(child);
+    }
+    is_child_on_edge = false;
+  }
+
   views::ColumnSet* columns = layout->AddColumnSet(0);
+
   if (IsHorizontalAlignment(alignment_)) {
     bool is_first_visible_child = true;
-    for (int c = 0; c < child_count(); ++c) {
+    for (int c = child_count() - 1; c >= 0; --c) {

[tdanderson, 2016/06/10 22:54:43]

Are you sure you still want to reverse the order of the iteration here?

[yiyix, 2016/06/13 18:43:55]

I think reverse the order makes more sense. With the loop ordering (int c = 0; c
< child_count(); ++c), we allocate the space on shelf for system trays with one
order, then we use a different order (int c = child_count() - 1; c >= 0; --c) in
the second for loop to use the space for another different tray. 

[yiyix, 2016/06/13 18:53:15]

Sorry, my explanation is not clear as I read it again. I want to say that with
the old approach, we use one order to allocate spacing in the layout, then we
use a different order to use the spacing. Ex: if child at index 0 is visible,
then we add the column c1 in the layout for this view. In the second loop, we
add logout button with index 4 (if logout button is visible) to the column c1. 

After changing the ordering in the first loop, we always add the view to the
column which is allocated for it.

       views::View* child = child_at(c);
       if (!child->visible())
         continue;
       if (!is_first_visible_child)
-        columns->AddPaddingColumn(0, kTraySpacing);
+        columns->AddPaddingColumn(0, GetTrayConstant(TRAY_SPACING));
       is_first_visible_child = false;
       columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::FILL,
                          0, /* resize percent */
                          views::GridLayout::USE_PREF, 0, 0);
     }
     layout->StartRow(0, 0);
     for (int c = child_count() - 1; c >= 0; --c) {
       views::View* child = child_at(c);
       if (child->visible())
         layout->AddView(child);
     }
   } else {
+    bool is_first_visible_child = true;
     columns->AddColumn(views::GridLayout::FILL, views::GridLayout::CENTER,
                        0, /* resize percent */
                        views::GridLayout::USE_PREF, 0, 0);
-    bool is_first_visible_child = true;
     for (int c = child_count() - 1; c >= 0; --c) {
       views::View* child = child_at(c);
       if (!child->visible())
         continue;
-      if (!is_first_visible_child)
-        layout->AddPaddingRow(0, kTraySpacing);
+      if (!is_first_visible_child) {
+        if (!((child_at(kOverviewButtonIndex)->visible()) &&
+              c == kOverviewButtonIndex)) {

[tdanderson, 2016/06/10 22:54:43]

Unless I am missing something, I think you can remove lines 163-164 since
they're no longer necessary. Be sure to remove the constant
|kOverviewButtonIndex| as well.

[yiyix, 2016/06/13 18:43:55]

Done.

+          layout->AddPaddingRow(0, GetTrayConstant(TRAY_SPACING));
+        }
+      }
       is_first_visible_child = false;
       layout->StartRow(0, 0);
       layout->AddView(child);
     }
   }
 
   layer()->GetAnimator()->StopAnimating();
   StatusAreaWidgetDelegateAnimationSettings settings(layer());
 
   Layout();
   UpdateWidgetSize();
 }
 
 void StatusAreaWidgetDelegate::ChildPreferredSizeChanged(View* child) {
   // Need to resize the window when trays or items are added/removed.
   StatusAreaWidgetDelegateAnimationSettings settings(layer());
   UpdateWidgetSize();
 }
 
 void StatusAreaWidgetDelegate::ChildVisibilityChanged(View* child) {
   UpdateLayout();
 }
 
 void StatusAreaWidgetDelegate::UpdateWidgetSize() {
   if (GetWidget())
     GetWidget()->SetSize(GetPreferredSize());
 }
 
+void StatusAreaWidgetDelegate::SetBorderOnEdgeTray(views::View* child) {
+  int top_edge, left_edge, bottom_edge, right_edge;
+
+  if (MaterialDesignController::IsShelfMaterial()) {
+    if (IsHorizontalAlignment(alignment_)) {
+      top_edge = (GetShelfConstant(SHELF_SIZE) - kShelfItemHeight) / 2;
+      left_edge = 0;
+      bottom_edge = (GetShelfConstant(SHELF_SIZE) - kShelfItemHeight) / 2;
+      right_edge = GetTrayConstant(PADDING_FROM_EDGE_OF_SHELF);
+    } else {
+      top_edge = 0;
+      left_edge = (GetShelfConstant(SHELF_SIZE) - kShelfItemHeight) / 2;
+      bottom_edge = GetTrayConstant(PADDING_FROM_EDGE_OF_SHELF);
+      right_edge = (GetShelfConstant(SHELF_SIZE) - kShelfItemHeight) / 2;
+    }
+  } else {
+    bool on_edge = (child == child_at(0));

[tdanderson, 2016/06/10 22:54:43]

Now that you're checking which child is on the edge in lines 118-130, I don't
think this variable is needed anymore.

To expand on my earlier suggestion I think 196-277 can be condensed into:

void SetBorderOnChild(View* child, bool extend_border_to_edge) {
  if (material) {
    ...
  } else {
    ... // Use |extend_border_to_edge| instead of |on_edge|
  }
}

[yiyix, 2016/06/13 18:43:54]

The only child with index 0 is overview button. In the non-md design, there is a
larger inset between overview and the edge of the screen. 

+    if (IsHorizontalAlignment(alignment_)) {
+      top_edge = kShelfItemInset;
+      left_edge = 0;
+      bottom_edge =
+          GetShelfConstant(SHELF_SIZE) - kShelfItemInset - kShelfItemHeight;
+      right_edge = on_edge ? GetTrayConstant(PADDING_FROM_EDGE_OF_SHELF) : 0;
+    } else if (alignment_ == SHELF_ALIGNMENT_LEFT) {
+      top_edge = 0;
+      left_edge =
+          GetShelfConstant(SHELF_SIZE) - kShelfItemInset - kShelfItemHeight;
+      bottom_edge = on_edge ? GetTrayConstant(PADDING_FROM_EDGE_OF_SHELF) : 0;
+      right_edge = kShelfItemInset;
+    } else {  // SHELF_ALIGNMENT_RIGHT
+      top_edge = 0;
+      left_edge = kShelfItemInset;
+      bottom_edge = on_edge ? GetTrayConstant(PADDING_FROM_EDGE_OF_SHELF) : 0;
+      right_edge =
+          GetShelfConstant(SHELF_SIZE) - kShelfItemInset - kShelfItemHeight;
+    }
+  }
+
+  child->SetBorder(views::Border::CreateEmptyBorder(top_edge, left_edge,
+                                                    bottom_edge, right_edge));
+}
+
+void StatusAreaWidgetDelegate::SetBorderOnNonEdgeTray(views::View* child) {
+  int top_edge, left_edge, bottom_edge, right_edge;
+  // Tray views are laid out right-to-left or bottom-to-top
+  if (MaterialDesignController::IsShelfMaterial()) {
+    if (IsHorizontalAlignment(alignment_)) {
+      top_edge = (GetShelfConstant(SHELF_SIZE) - kShelfItemHeight) / 2;
+      left_edge = 0;
+      bottom_edge = (GetShelfConstant(SHELF_SIZE) - kShelfItemHeight) / 2;
+      right_edge = 0;
+    } else {
+      top_edge = 0;
+      left_edge = (GetShelfConstant(SHELF_SIZE) - kShelfItemHeight) / 2;
+      bottom_edge = 0;
+      right_edge = (GetShelfConstant(SHELF_SIZE) - kShelfItemHeight) / 2;
+    }
+  } else {
+    bool on_edge = (child == child_at(0));
+    if (IsHorizontalAlignment(alignment_)) {
+      top_edge = kShelfItemInset;
+      left_edge = 0;
+      bottom_edge =
+          GetShelfConstant(SHELF_SIZE) - kShelfItemInset - kShelfItemHeight;
+      right_edge = on_edge ? GetTrayConstant(PADDING_FROM_EDGE_OF_SHELF) : 0;
+    } else if (alignment_ == SHELF_ALIGNMENT_LEFT) {
+      top_edge = 0;
+      left_edge =
+          GetShelfConstant(SHELF_SIZE) - kShelfItemInset - kShelfItemHeight;
+      bottom_edge = on_edge ? GetTrayConstant(PADDING_FROM_EDGE_OF_SHELF) : 0;
+      right_edge = kShelfItemInset;
+    } else {  // SHELF_ALIGNMENT_RIGHT
+      top_edge = 0;
+      left_edge = kShelfItemInset;
+      bottom_edge = on_edge ? GetTrayConstant(PADDING_FROM_EDGE_OF_SHELF) : 0;
+      right_edge =
+          GetShelfConstant(SHELF_SIZE) - kShelfItemInset - kShelfItemHeight;
+    }
+  }
+  child->SetBorder(views::Border::CreateEmptyBorder(top_edge, left_edge,
+                                                    bottom_edge, right_edge));
+}
+
 }  // namespace ash