ios/clean/chrome/browser/ui/tools/menu_overflow_controls_stackview.h - Issue 2693043002: [ios clean] Add overflow buttons for ToolsMenu.

Side by Side Diff: ios/clean/chrome/browser/ui/tools/menu_overflow_controls_stackview.h

Issue 2693043002: [ios clean] Add overflow buttons for ToolsMenu. (Closed)

Patch Set: Created 3 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« ios/clean/chrome/browser/ui/toolbar/toolbar_view_controller.mm ('K') | « ios/clean/chrome/browser/ui/tools/BUILD.gn ('k') | ios/clean/chrome/browser/ui/tools/menu_overflow_controls_stackview.mm » ('j') | ios/clean/chrome/browser/ui/tools/menu_view_controller.mm » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright 2017 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef IOS_CLEAN_CHROME_BROWSER_UI_TOOLS_MENU_OVERFLOW_CONTROLS_STACKVIEW_H_

	6 #define IOS_CLEAN_CHROME_BROWSER_UI_TOOLS_MENU_OVERFLOW_CONTROLS_STACKVIEW_H_

	7

	8 #import <UIKit/UIKit.h>

	9

	10 @class ToolbarButton;

	11

	12 // StackView subclass that contains the Overflow Toolbar Buttons that will be

	13 // inserted in the first row of ToolMenu in compact widths.

	14 @interface MenuOverflowControlsStackView : UIStackView
	edchin 2017/02/15 04:14:50 I would rather avoid inheritance in this case. Th I would rather avoid inheritance in this case. There is nothing special about MenuOverflowControlsStackView that warrants the use of inheritance. Instead, you can use a category that constructs a custom stackView. UIStackView +(instancetype)cr_menuOverflowControlsStackView. In this factory method, you can construct the buttons and also set their actions. marq (ping after 24h) 2017/02/15 15:06:16 +1, this is the pattern we've been using so far. E Show quoted text On 2017/02/15 04:14:50, edchin wrote: > I would rather avoid inheritance in this case. > There is nothing special about MenuOverflowControlsStackView > that warrants the use of inheritance. > Instead, you can use a category that constructs > a custom stackView. > UIStackView +(instancetype)cr_menuOverflowControlsStackView. > In this factory method, you can construct the buttons > and also set their actions. +1, this is the pattern we've been using so far. Even if we change our minds later, it's easier if we're consistent. sczs 2017/02/16 00:31:39 The Buttons visibility needs to be set by the Tool Show quoted text On 2017/02/15 15:06:16, marq wrote: > On 2017/02/15 04:14:50, edchin wrote: > > I would rather avoid inheritance in this case. > > There is nothing special about MenuOverflowControlsStackView > > that warrants the use of inheritance. > > Instead, you can use a category that constructs > > a custom stackView. > > UIStackView +(instancetype)cr_menuOverflowControlsStackView. > > In this factory method, you can construct the buttons > > and also set their actions. > > +1, this is the pattern we've been using so far. Even if we change our minds > later, it's easier if we're consistent. The Buttons visibility needs to be set by the ToolsMenuVCn (Stop/Reload, Starred/Unstarred), that's why I need a reference to the buttons, and that was one of the reasons why I was subclassing. If we want to keep using categories we can create associated objects, or we can keep this subclass. I had a quick chat with Ed and he thought that this might enough to keep this as a subclass, WDYT? edchin 2017/02/16 01:21:00 Agreed. After some more discussion, there's additi Show quoted text On 2017/02/16 00:31:39, sczs wrote: > On 2017/02/15 15:06:16, marq wrote: > > On 2017/02/15 04:14:50, edchin wrote: > > > I would rather avoid inheritance in this case. > > > There is nothing special about MenuOverflowControlsStackView > > > that warrants the use of inheritance. > > > Instead, you can use a category that constructs > > > a custom stackView. > > > UIStackView +(instancetype)cr_menuOverflowControlsStackView. > > > In this factory method, you can construct the buttons > > > and also set their actions. > > > > +1, this is the pattern we've been using so far. Even if we change our minds > > later, it's easier if we're consistent. > > The Buttons visibility needs to be set by the ToolsMenuVCn (Stop/Reload, > Starred/Unstarred), that's why I need a reference to the buttons, and that was > one of the reasons why I was subclassing. > If we want to keep using categories we can create associated objects, or we can > keep this subclass. I had a quick chat with Ed and he thought that this might > enough to keep this as a subclass, WDYT? Agreed. After some more discussion, there's additional functionality in the menu overflow controls that requires subclassing.
	15 // ToolsMenu ToolbarButton.

	16 @property(nonatomic, strong) ToolbarButton* toolsMenuButton;

	17 // Share ToolbarButton.

	18 @property(nonatomic, strong) ToolbarButton* shareButton;

	19 // Reload ToolbarButton.

	20 @property(nonatomic, strong) ToolbarButton* reloadButton;

	21 // Stop ToolbarButton.

	22 @property(nonatomic, strong) ToolbarButton* stopButton;

	23 @end

	24

	25 #endif // IOS_CLEAN_CHROME_BROWSER_UI_TOOLS_MENU_OVERFLOW_CONTROLS_STACKVIEW_H_

OLD	NEW