Refactor AddScrollListItem() in system menu detailed views

ash/system/tray/tray_details_view.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 ASH_SYSTEM_TRAY_TRAY_DETAILS_VIEW_H_
 #define ASH_SYSTEM_TRAY_TRAY_DETAILS_VIEW_H_
 
 #include <memory>
 
 #include "ash/ash_export.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/view_click_listener.h"
 #include "base/macros.h"
 #include "base/timer/timer.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/view.h"
 
+namespace gfx {
+struct VectorIcon;
+}  // namespace gfx
+
 namespace views {
 class BoxLayout;
 class CustomButton;
 class ProgressBar;
 class ScrollView;
 }  // namespace views
 
 namespace ash {
 namespace test {
 class TrayDetailsViewTest;
 }  // namespace test
 
+class HoverHighlightView;
 class ScrollBorder;
 class SystemTrayItem;
 class TriView;
 
 class ASH_EXPORT TrayDetailsView : public views::View,
                                    public ViewClickListener,
                                    public views::ButtonListener {
  public:
   explicit TrayDetailsView(SystemTrayItem* owner);
   ~TrayDetailsView() override;
 
   // ViewClickListener:
   // Don't override this --- override HandleViewClicked.
   void OnViewClicked(views::View* sender) final;
 
   // views::ButtonListener:
   // Don't override this --- override HandleButtonPressed.
   void ButtonPressed(views::Button* sender, const ui::Event& event) final;
 
   SystemTrayItem* owner() { return owner_; }
-  views::ScrollView* scroller() { return scroller_; }
-  views::View* scroll_content() { return scroll_content_; }
 
  protected:
   // views::View:
   void Layout() override;
   int GetHeightForWidth(int width) const override;
 
   // Exposes the layout manager of this view to give control to subclasses.
   views::BoxLayout* box_layout() { return box_layout_; }
 
   // Creates the row containing the back button and title. For material design
   // this appears at the top of the view, for non-material design it appears
   // at the bottom.
   void CreateTitleRow(int string_id);
 
   // Creates a scrollable list. The list has a border at the bottom if there is
   // any other view between the list and the footer row at the bottom.
   void CreateScrollableList();
 
+  // Adds an item to the scroll list. |gfx::kNoneIcon| can be used if there is

[tdanderson, 2017/04/24 15:43:39]

nit: consider a rewording of first sentence to give a bit more information such
as "Creates a targetable row containing |icon| and |text| and adds it as a child
view of |scroll_content_|" or similar. Ditto for the other docs here.

[mohsen, 2017/04/25 05:10:01]

Done.

+  // no icon.
+  HoverHighlightView* AddScrollListItem(const gfx::VectorIcon& icon,
+                                        const base::string16& text);
+  // Adds an item to the scroll list with a checkbox. |gfx::kNoneIcon| can be
+  // used if there is no icon.
+  HoverHighlightView* AddScrollListCheckableItem(const gfx::VectorIcon& icon,
+                                                 const base::string16& text,
+                                                 bool checked);

[tdanderson, 2017/04/24 15:43:39]

nit: newline

[mohsen, 2017/04/25 05:10:01]

Done.

+  // Adds a sticky sub header to the scroll list. |gfx::kNoneIcon| can be used
+  // if there is no icon.
+  TriView* AddScrollListSubHeader(const gfx::VectorIcon& icon,

[tdanderson, 2017/04/24 15:43:39]

To simplify the call sites (not requiring kNoneIcon to be passed in for certain
cases), consider having both AddScrollListSubHeader(icon) and
AddScrollListSubHeader(icon,text) protected methods here. Ditto for
AddScrollListItem().

[mohsen, 2017/04/25 05:10:01]

Done.

+                                  const base::string16& text);
+
   // Removes (and destroys) all child views.
   void Reset();
 
   // Shows or hides the progress bar below the title row. It occupies the same
   // space as the separator, so when shown the separator is hidden. If
   // |progress_bar_| doesn't already exist it will be created.
   void ShowProgress(double value, bool visible);
 
   // Helper functions which create and return the settings and help buttons,
   // respectively, used in the material design top-most header row. The caller
   // assumes ownership of the returned buttons.
   views::CustomButton* CreateSettingsButton(int setting_accessible_name_id);
   views::CustomButton* CreateHelpButton();
 
   TriView* tri_view() { return tri_view_; }
+  views::ScrollView* scroller() const { return scroller_; }
+  views::View* scroll_content() const { return scroll_content_; }
 
  private:
   friend class test::TrayDetailsViewTest;
 
   // Overridden to handle clicks on subclass-specific views.
   virtual void HandleViewClicked(views::View* view);
 
   // Overridden to handle button presses on subclass-specific buttons.
   virtual void HandleButtonPressed(views::Button* sender,
                                    const ui::Event& event);
@@ skipping 33 matching lines @@
 
   // Used to delay the transition to the default view.
   base::OneShotTimer transition_delay_timer_;
 
   DISALLOW_COPY_AND_ASSIGN(TrayDetailsView);
 };
 
 }  // namespace ash
 
 #endif  // ASH_SYSTEM_TRAY_TRAY_DETAILS_VIEW_H_