remoting/ios/app/app_view_controller.h - Issue 2937733003: [CRD iOS] Branching internal and external implementations

Side by Side Diff: remoting/ios/app/app_view_controller.h

Issue 2937733003: [CRD iOS] Branching internal and external implementations (Closed)

Patch Set: Add the requestSignIn method Created 3 years, 6 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:

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 REMOTING_IOS_APP_APP_VIEW_CONTROLLER_H_

	6 #define REMOTING_IOS_APP_APP_VIEW_CONTROLLER_H_

	7

	8 #import <UIKit/UIKit.h>

	9

	10 // The root view controller of the app. It acts as a container controller for

	11 // \|mainViewController\|, which will be shown as the primary content of the view.

	12 // Implementation can add drawer or modal for things like the side menu or help

	13 // and feedback.

	14 @interface AppViewController : UIViewController

	15

	16 // For adding new method, please make sure the method is implemented privatel
	nicholss 2017/06/19 16:32:09 Fix typos: "methods" or "a new method", "privately Fix typos: "methods" or "a new method", "privately" To ensure there is a mirror of methods you can make a protocol. setMenuVisible is not a good iOS name. It should be two methods showMenuAnimated: and hideMenuAnimated:. The set verb is normally only used for setters. This implies that you are setting the property "menuVisible". Yuwei 2017/06/19 19:17:26 Done. Show quoted text On 2017/06/19 16:32:09, nicholss wrote: > Fix typos: "methods" or "a new method", "privately" > > To ensure there is a mirror of methods you can make a protocol. > > setMenuVisible is not a good iOS name. It should be two methods > showMenuAnimated: and hideMenuAnimated:. The set verb is normally only used for > setters. This implies that you are setting the property "menuVisible". Done.
	17 // on both ios_internal and Chromium before adding the method to the interface

	18 // and hooking it up to the caller. For deletion, just reverse the procedure.

	19 // This is to prevent build break in internal buildbot.

	20

	21 - (void)setMenuVisible:(BOOL)visible animated:(BOOL)animated;

	22

	23 - (void)requestSignIn;
	nicholss 2017/06/19 16:32:10 I don't think you mean request, this implies that I don't think you mean request, this implies that there will be a call to the sign-in server, I think you mean presentSignIn. In iOS the naming matters a lot for clarity. In usage the methods are suppose to read like language. Yuwei 2017/06/19 19:17:26 Done. Show quoted text On 2017/06/19 16:32:10, nicholss wrote: > I don't think you mean request, this implies that there will be a call to the > sign-in server, I think you mean presentSignIn. > > In iOS the naming matters a lot for clarity. In usage the methods are suppose to > read like language. Done.
	24

	25 @property(nonatomic) UIViewController* mainViewController;
	nicholss 2017/06/19 16:32:10 mainViewController should be readonly, but I don't mainViewController should be readonly, but I don't understand why you need this. Yuwei 2017/06/19 19:17:26 This is used to set the main VC to be shown on App Show quoted text On 2017/06/19 16:32:10, nicholss wrote: > mainViewController should be readonly, but I don't understand why you need this. This is used to set the main VC to be shown on AppViewController. I moved the assignment into the init method to simplify it.
	26

	27 @end

	28

	29 #endif // REMOTING_IOS_APP_APP_VIEW_CONTROLLER_H_

OLD	NEW

« no previous file with comments | « remoting/ios/app/app_delegate.mm ('k') | remoting/ios/app/app_view_controller_chromium.mm » ('j') | remoting/ios/app/app_view_controller_chromium.mm » ('J')