The touch menu for ChromeOS will be used for all flavors of ChromeOS while other operating systems should stay as they are.

Furthermore changed according to specifications which can be seen in the issue.

Cleaned up the menu separator constants.


BUG=137345
TEST=visually checked

Review URL: https://chromiumcodereview.appspot.com/10837317

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@153157 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 87 insertions, 34 deletions

diff --git a/ui/views/controls/menu/menu_config.cc b/ui/views/controls/menu/menu_config.cc
index 55ad7a0..8cd3a8f 100644
--- a/ui/views/controls/menu/menu_config.cc
+++ b/ui/views/controls/menu/menu_config.cc
@@ -33,6 +33,9 @@ MenuConfig::MenuConfig()
       arrow_width(9),
       gutter_width(0),
       separator_height(6),
+      separator_upper_height(3),
+      separator_lower_height(4),
+      separator_spacing_height(3),
       render_gutter(false),
       show_mnemonics(false),
       scroll_arrow_height(3),
@@ -43,6 +46,7 @@ MenuConfig::MenuConfig()
       align_arrow_and_shortcut(false),
       offset_context_menus(false) {
   // Use 40px tall menu items when running in touch optimized mode.
+  // For Windows use 40px tall menu items when running in touch optimized mode.
   if (ui::GetDisplayLayout() == ui::LAYOUT_TOUCH) {
     item_top_margin = item_no_icon_top_margin = 12;
     item_bottom_margin = item_no_icon_bottom_margin = 13;
diff --git a/ui/views/controls/menu/menu_config.h b/ui/views/controls/menu/menu_config.h
index cce8673..32d26e7 100644
--- a/ui/views/controls/menu/menu_config.h
+++ b/ui/views/controls/menu/menu_config.h
@@ -81,9 +81,18 @@ struct VIEWS_EXPORT MenuConfig {
   // Width of the gutter. Only used if render_gutter is true.
   int gutter_width;
 
-  // Height of the separator.
+  // Height of a normal separator (ui::NORMAL_SEPARATOR).
   int separator_height;
 
+  // Height of a ui::UPPER_SEPARATOR.
+  int separator_upper_height;
+
+  // Height of a ui::LOWER_SEPARATOR.
+  int separator_lower_height;
+
+  // Height of a ui::SPACING_SEPARATOR.
+  int separator_spacing_height;
+
   // Whether or not the gutter should be rendered. The gutter is specific to
   // Vista.
   bool render_gutter;
diff --git a/ui/views/controls/menu/menu_config_views.cc b/ui/views/controls/menu/menu_config_views.cc
index d6943d4..e369c956 100644
--- a/ui/views/controls/menu/menu_config_views.cc
+++ b/ui/views/controls/menu/menu_config_views.cc
@@ -31,13 +31,7 @@ MenuConfig* MenuConfig::Create() {
   config->check_height = check->height();
   config->item_left_margin = 4;
   config->item_min_height = 29;
-  // TODO(skuhne): Separators in menus should have the same padding in touch
-  // mode but currently the touch wrench menu relies on separators having no
-  // padding to implement full-height buttons.
-  if (ui::GetDisplayLayout() == ui::LAYOUT_TOUCH)
-    config->separator_height = 1;
-  else
-    config->separator_height = 15;
+  config->separator_height = 15;
   config->font = rb.GetFont(ResourceBundle::BaseFont);
   config->label_to_arrow_padding = 20;
   config->label_to_accelerator_padding = 20;
diff --git a/ui/views/controls/menu/menu_item_view.cc b/ui/views/controls/menu/menu_item_view.cc
index cae0382..bc0c1f7 100644
--- a/ui/views/controls/menu/menu_item_view.cc
+++ b/ui/views/controls/menu/menu_item_view.cc
@@ -206,18 +206,20 @@ void MenuItemView::Cancel() {
   }
 }
 
-MenuItemView* MenuItemView::AddMenuItemAt(int index,
-                                          int item_id,
-                                          const string16& label,
-                                          const gfx::ImageSkia& icon,
-                                          Type type) {
+MenuItemView* MenuItemView::AddMenuItemAt(
+    int index,
+    int item_id,
+    const string16& label,
+    const gfx::ImageSkia& icon,
+    Type type,
+    ui::MenuSeparatorType separator_style) {
   DCHECK_NE(type, EMPTY);
   DCHECK_LE(0, index);
   if (!submenu_)
     CreateSubmenu();
   DCHECK_GE(submenu_->child_count(), index);
   if (type == SEPARATOR) {
-    submenu_->AddChildViewAt(new MenuSeparator(), index);
+    submenu_->AddChildViewAt(new MenuSeparator(separator_style), index);
     return NULL;
   }
   MenuItemView* item = new MenuItemView(this, item_id, type);
@@ -251,18 +253,21 @@ void MenuItemView::RemoveMenuItemAt(int index) {
 MenuItemView* MenuItemView::AppendMenuItem(int item_id,
                                            const string16& label,
                                            Type type) {
-  return AppendMenuItemImpl(item_id, label, gfx::ImageSkia(), type);
+  return AppendMenuItemImpl(item_id, label, gfx::ImageSkia(), type,
+      ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendSubMenu(int item_id,
                                           const string16& label) {
-  return AppendMenuItemImpl(item_id, label, gfx::ImageSkia(), SUBMENU);
+  return AppendMenuItemImpl(item_id, label, gfx::ImageSkia(), SUBMENU,
+      ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendSubMenuWithIcon(int item_id,
                                                   const string16& label,
                                                   const gfx::ImageSkia& icon) {
-  return AppendMenuItemImpl(item_id, label, icon, SUBMENU);
+  return AppendMenuItemImpl(
+      item_id, label, icon, SUBMENU, ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendMenuItemWithLabel(int item_id,
@@ -275,13 +280,15 @@ MenuItemView* MenuItemView::AppendDelegateMenuItem(int item_id) {
 }
 
 void MenuItemView::AppendSeparator() {
-  AppendMenuItemImpl(0, string16(), gfx::ImageSkia(), SEPARATOR);
+  AppendMenuItemImpl(
+      0, string16(), gfx::ImageSkia(), SEPARATOR, ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendMenuItemWithIcon(int item_id,
                                                    const string16& label,
                                                    const gfx::ImageSkia& icon) {
-  return AppendMenuItemImpl(item_id, label, icon, NORMAL);
+  return AppendMenuItemImpl(
+      item_id, label, icon, NORMAL, ui::NORMAL_SEPARATOR);
 }
 
 MenuItemView* MenuItemView::AppendMenuItemFromModel(ui::MenuModel* model,
@@ -289,6 +296,7 @@ MenuItemView* MenuItemView::AppendMenuItemFromModel(ui::MenuModel* model,
                                                     int id) {
   gfx::Image icon;
   string16 label;
+  ui::MenuSeparatorType separator_style = ui::NORMAL_SEPARATOR;
   MenuItemView::Type type;
   ui::MenuModel::ItemType menu_type = model->GetTypeAt(index);
   switch (menu_type) {
@@ -307,6 +315,7 @@ MenuItemView* MenuItemView::AppendMenuItemFromModel(ui::MenuModel* model,
       break;
     case ui::MenuModel::TYPE_SEPARATOR:
       type = MenuItemView::SEPARATOR;
+      separator_style = model->GetSeparatorTypeAt(index);
       break;
     case ui::MenuModel::TYPE_SUBMENU:
       model->GetIconAt(index, &icon);
@@ -322,15 +331,18 @@ MenuItemView* MenuItemView::AppendMenuItemFromModel(ui::MenuModel* model,
   return AppendMenuItemImpl(id,
       label,
       icon.IsEmpty() ? gfx::ImageSkia() : *icon.ToImageSkia(),
-      type);
+      type,
+      separator_style);
 }
 
-MenuItemView* MenuItemView::AppendMenuItemImpl(int item_id,
-                                               const string16& label,
-                                               const gfx::ImageSkia& icon,
-                                               Type type) {
+MenuItemView* MenuItemView::AppendMenuItemImpl(
+    int item_id,
+    const string16& label,
+    const gfx::ImageSkia& icon,
+    Type type,
+    ui::MenuSeparatorType separator_style) {
   const int index = submenu_ ? submenu_->child_count() : 0;
-  return AddMenuItemAt(index, item_id, label, icon, type);
+  return AddMenuItemAt(index, item_id, label, icon, type, separator_style);
 }
 
 SubmenuView* MenuItemView::CreateSubmenu() {
diff --git a/ui/views/controls/menu/menu_item_view.h b/ui/views/controls/menu/menu_item_view.h
index c80a1d3..bbe258c 100644
--- a/ui/views/controls/menu/menu_item_view.h
+++ b/ui/views/controls/menu/menu_item_view.h
@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/string16.h"
 #include "build/build_config.h"
+#include "ui/base/models/menu_separator_types.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/views/view.h"
 
@@ -133,7 +134,8 @@ class VIEWS_EXPORT MenuItemView : public View {
                               int item_id,
                               const string16& label,
                               const gfx::ImageSkia& icon,
-                              Type type);
+                              Type type,
+                              ui::MenuSeparatorType separator_style);
 
   // Remove an item from the menu at a specified index.
   // ChildrenChanged() should be called after removing menu items (whether
@@ -193,7 +195,8 @@ class VIEWS_EXPORT MenuItemView : public View {
   MenuItemView* AppendMenuItemImpl(int item_id,
                                    const string16& label,
                                    const gfx::ImageSkia& icon,
-                                   Type type);
+                                   Type type,
+                                   ui::MenuSeparatorType separator_style);
 
   // Returns the view that contains child menu items. If the submenu has
   // not been creates, this creates it.
diff --git a/ui/views/controls/menu/menu_model_adapter_unittest.cc b/ui/views/controls/menu/menu_model_adapter_unittest.cc
index a371d9a..ee0846f 100644
--- a/ui/views/controls/menu/menu_model_adapter_unittest.cc
+++ b/ui/views/controls/menu/menu_model_adapter_unittest.cc
@@ -50,6 +50,11 @@ class MenuModelBase : public ui::MenuModel {
     return items_[index - GetFirstItemIndex(NULL)].type;
   }
 
+  virtual ui::MenuSeparatorType GetSeparatorTypeAt(
+      int index) const OVERRIDE {
+    return ui::NORMAL_SEPARATOR;
+  }
+
   virtual int GetCommandIdAt(int index) const OVERRIDE {
     return index - GetFirstItemIndex(NULL) + command_id_base_;
   }
diff --git a/ui/views/controls/menu/menu_separator.h b/ui/views/controls/menu/menu_separator.h
index f3eabb2..cb48e04 100644
--- a/ui/views/controls/menu/menu_separator.h
+++ b/ui/views/controls/menu/menu_separator.h
@@ -6,19 +6,23 @@
 #define UI_VIEWS_CONTROLS_MENU_MENU_SEPARATOR_H_
 
 #include "base/compiler_specific.h"
+#include "ui/base/models/menu_separator_types.h"
 #include "ui/views/view.h"
 
 namespace views {
 
 class MenuSeparator : public View {
  public:
-  MenuSeparator() {}
+  explicit MenuSeparator(ui::MenuSeparatorType type) : type_(type) {}
 
   // View overrides.
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
   virtual gfx::Size GetPreferredSize() OVERRIDE;
 
  private:
+  // The type of the separator.
+  const ui::MenuSeparatorType type_;
+
   DISALLOW_COPY_AND_ASSIGN(MenuSeparator);
 };
 
diff --git a/ui/views/controls/menu/menu_separator_views.cc b/ui/views/controls/menu/menu_separator_views.cc
index 61ec07d..4d267df 100644
--- a/ui/views/controls/menu/menu_separator_views.cc
+++ b/ui/views/controls/menu/menu_separator_views.cc
@@ -18,14 +18,36 @@ const int kSeparatorHeight = 1;
 namespace views {
 
 void MenuSeparator::OnPaint(gfx::Canvas* canvas) {
-  canvas->FillRect(gfx::Rect(0, height() / 2, width(), kSeparatorHeight),
+  int pos = 0;
+  if (type_ == ui::LOWER_SEPARATOR)
+    pos = height() - kSeparatorHeight;
+  else if (type_ != ui::SPACING_SEPARATOR)
+    pos = height() / 2;
+  else if (type_ != ui::UPPER_SEPARATOR)
+    return;
+  canvas->FillRect(gfx::Rect(0, pos, width(), kSeparatorHeight),
       ui::NativeTheme::instance()->GetSystemColor(
-          ui::NativeTheme::kColorId_MenuSeparatorColor));
+            ui::NativeTheme::kColorId_MenuSeparatorColor));
 }
 
 gfx::Size MenuSeparator::GetPreferredSize() {
+  int height = MenuConfig::instance().separator_height;
+  switch(type_) {
+    case ui::SPACING_SEPARATOR:
+      height = MenuConfig::instance().separator_spacing_height;
+      break;
+    case ui::LOWER_SEPARATOR:
+      height = MenuConfig::instance().separator_lower_height;
+      break;
+    case ui::UPPER_SEPARATOR:
+      height = MenuConfig::instance().separator_upper_height;
+      break;
+    default:
+      height = MenuConfig::instance().separator_height;
+      break;
+  }
   return gfx::Size(10,  // Just in case we're the only item in a menu.
-                   MenuConfig::instance().separator_height);
+                   height);
 }
 
 }  // namespace views
diff --git a/ui/views/controls/textfield/native_textfield_views.cc b/ui/views/controls/textfield/native_textfield_views.cc
index 71c795f85..c1afcea 100644
--- a/ui/views/controls/textfield/native_textfield_views.cc
+++ b/ui/views/controls/textfield/native_textfield_views.cc
@@ -1069,7 +1069,7 @@ void NativeTextfieldViews::UpdateContextMenu() {
     context_menu_contents_->AddItemWithStringId(IDS_APP_COPY, IDS_APP_COPY);
     context_menu_contents_->AddItemWithStringId(IDS_APP_PASTE, IDS_APP_PASTE);
     context_menu_contents_->AddItemWithStringId(IDS_APP_DELETE, IDS_APP_DELETE);
-    context_menu_contents_->AddSeparator();
+    context_menu_contents_->AddSeparator(ui::NORMAL_SEPARATOR);
     context_menu_contents_->AddItemWithStringId(IDS_APP_SELECT_ALL,
                                                 IDS_APP_SELECT_ALL);
     TextfieldController* controller = textfield_->GetController();
diff --git a/ui/views/controls/textfield/native_textfield_win.cc b/ui/views/controls/textfield/native_textfield_win.cc
index a89896c..6baebf6 100644
--- a/ui/views/controls/textfield/native_textfield_win.cc
+++ b/ui/views/controls/textfield/native_textfield_win.cc
@@ -1197,11 +1197,11 @@ void NativeTextfieldWin::BuildContextMenu() {
     return;
   context_menu_contents_.reset(new ui::SimpleMenuModel(this));
   context_menu_contents_->AddItemWithStringId(IDS_APP_UNDO, IDS_APP_UNDO);
-  context_menu_contents_->AddSeparator();
+  context_menu_contents_->AddSeparator(ui::NORMAL_SEPARATOR);
   context_menu_contents_->AddItemWithStringId(IDS_APP_CUT, IDS_APP_CUT);
   context_menu_contents_->AddItemWithStringId(IDS_APP_COPY, IDS_APP_COPY);
   context_menu_contents_->AddItemWithStringId(IDS_APP_PASTE, IDS_APP_PASTE);
-  context_menu_contents_->AddSeparator();
+  context_menu_contents_->AddSeparator(ui::NORMAL_SEPARATOR);
   context_menu_contents_->AddItemWithStringId(IDS_APP_SELECT_ALL,
                                               IDS_APP_SELECT_ALL);
 }