MenuButton: support Mac look & feel

ui/views/controls/combobox/combobox.h

 // 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.
 
 #ifndef UI_VIEWS_CONTROLS_COMBOBOX_COMBOBOX_H_
 #define UI_VIEWS_CONTROLS_COMBOBOX_COMBOBOX_H_
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
@@ skipping 36 matching lines @@
   enum Style {
     STYLE_NORMAL,
     STYLE_ACTION,
   };
 
   // The combobox's class name.
   static const char kViewClassName[];
 
   // |model| is not owned by the combobox.
   explicit Combobox(ui::ComboboxModel* model);
+  Combobox(ui::ComboboxModel* model, Style style);

[tapted, 2016/04/26 05:05:58]

nit: move before the `explicit` constructor? Or reorder methods in .cc. Or (my
favourite): give `style` a default argument in the definition here (and add
`explicit` to this method). Default arguments are allowed now and this is a
pretty clear-cut case. It's good intrinsic documentation about exactly what
style you will get if you don't specify one.

[Elly Fong-Jones, 2016/04/26 17:41:25]

Done.

   ~Combobox() override;
 
   static const gfx::FontList& GetFontList();
 
   // Sets the listener which will be called when a selection has been made.
   void set_listener(ComboboxListener* listener) { listener_ = listener; }
 
-  void SetStyle(Style style);
-
   // Informs the combobox that its model changed.
   void ModelChanged();
 
   // Gets/Sets the selected index.
   int selected_index() const { return selected_index_; }
   void SetSelectedIndex(int index);
 
   // Looks for the first occurrence of |value| in |model()|. If found, selects
   // the found index and returns true. Otherwise simply noops and returns false.
   bool SelectValue(const base::string16& value);
 
   ui::ComboboxModel* model() const { return model_; }
 
   // Set the accessible name of the combobox.
   void SetAccessibleName(const base::string16& name);
 
   // Visually marks the combobox as having an invalid value selected.
   // When invalid, it paints with white text on a red background.
   // Callers are responsible for restoring validity with selection changes.
   void SetInvalid(bool invalid);
   bool invalid() const { return invalid_; }
 
-  // Returns the width of the arrow button component of the combobox: the arrow
-  // button itself, and the padding on either side of it.
-  int GetArrowButtonWidth() const;
+  // Returns the width of the combobox's "shoulder", which is the part
+  // containing the arrow, including any padding space around it.

[tapted, 2016/04/26 05:05:57]

nit: remove "space"

[Elly Fong-Jones, 2016/04/26 17:41:26]

Done.

+  int GetShoulderWidth() const;

[tapted, 2016/04/26 05:05:57]

Can these be in the private: section? Also make sure the method order in the .cc
matches

[Elly Fong-Jones, 2016/04/26 17:41:26]

Done.

 
   // Overridden from View:
   gfx::Size GetPreferredSize() const override;
   const char* GetClassName() const override;
   bool SkipDefaultKeyEventProcessing(const ui::KeyEvent& e) override;
   bool OnKeyPressed(const ui::KeyEvent& e) override;
   bool OnKeyReleased(const ui::KeyEvent& e) override;
   void OnPaint(gfx::Canvas* canvas) override;
   void OnFocus() override;
   void OnBlur() override;
@@ skipping 44 matching lines @@
 
   // Handles the clicking event.
   void HandleClickEvent();
 
   PrefixSelector* GetPrefixSelector();
 
   // Our model. Not owned.
   ui::ComboboxModel* model_;
 
   // The visual style of this combobox.
-  Style style_;
+  const Style style_;

[tapted, 2016/04/26 05:05:57]

nice - I like this :)

[Elly Fong-Jones, 2016/04/26 17:41:26]

Done.

 
   // Our listener. Not owned. Notified when the selected index change.
   ComboboxListener* listener_;
 
   // The current selected index; -1 and means no selection.
   int selected_index_;
 
   // True when the selection is visually denoted as invalid.
   bool invalid_;
 
@@ skipping 30 matching lines @@
   // STYLE_NOTIFY_ON_CLICK, a Combobox renders the button images according to
   // these button states.
   // The base View takes the ownerships of these as child views.
   CustomButton* text_button_;
   CustomButton* arrow_button_;
 
   // Set while the dropdown is showing. Ensures the menu is closed if |this| is
   // destroyed.
   std::unique_ptr<views::MenuRunner> menu_runner_;
 
+  // The image to be drawn for this combobox's arrow.
+  gfx::ImageSkia enabled_arrow_image_;
+  gfx::ImageSkia disabled_arrow_image_;

[tapted, 2016/04/26 05:05:57]

(per prior comment, just one image)

[Elly Fong-Jones, 2016/04/26 17:41:26]

Done.

+
   // Used for making calbacks.
   base::WeakPtrFactory<Combobox> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Combobox);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_CONTROLS_COMBOBOX_COMBOBOX_H_