Move the fake omnibox constraints out of GoogleLandingViewController

Move the fake omnibox constraints out of GoogleLandingViewController

In order to refactor GoogleLandingViewController, the configuration of the
elements displayed in the fake omnibox are moved to a helper.

BUG=717974
Review-Url: https://codereview.chromium.org/2855113003
Cr-Commit-Position: refs/heads/master@{#470583}
Committed: https://chromium.googlesource.com/chromium/src/+/02b01701047edf186f7d4164af8bcc6a0c78594f

[gambard, 2017-05-03 14:13:33]

gambard@chromium.org changed reviewers:
+ marq@chromium.org

[gambard, 2017-05-03 14:13:33]

PTAL.

[marq (ping after 24h), 2017-05-03 15:22:06]

lgtm

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h
(right):

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h:31:
// Returns a UILabel containing the hint for the omnibox, added to the
Normally I would object to a function that does this kind of thing, but since
this is in the context of major refactoring, I'm fine with it for the time
being.

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h:35:
// Returns a button for the voice search, added to the |searcgTapTarget| and
searcg -> search

[gambard, 2017-05-03 15:27:35]

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h
(right):

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h:31:
// Returns a UILabel containing the hint for the omnibox, added to the
On 2017/05/03 15:22:05, marq (ping after 24h) wrote:
> Normally I would object to a function that does this kind of thing, but since
> this is in the context of major refactoring, I'm fine with it for the time
> being.

Which part are you disagreeing on? My first intent was to create the object and
add it to the searchTapTarget outside this helper, then only configure it
(including the constraint) in the helper. Would that be more acceptable?

[marq (ping after 24h), 2017-05-10 09:11:26]

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h
(right):

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h:31:
// Returns a UILabel containing the hint for the omnibox, added to the
On 2017/05/03 15:27:35, gambard wrote:
> On 2017/05/03 15:22:05, marq (ping after 24h) wrote:
> > Normally I would object to a function that does this kind of thing, but
since
> > this is in the context of major refactoring, I'm fine with it for the time
> > being.
> 
> Which part are you disagreeing on? My first intent was to create the object
and
> add it to the searchTapTarget outside this helper, then only configure it
> (including the constraint) in the helper. Would that be more acceptable?

Yes; in general I don't like functions that return values and also have side
effects, so having a utility that just arranges existing views passed into it
seems cleaner.

[gambard, 2017-05-10 12:14:44]

Thanks!

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
File
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h
(right):

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h:31:
// Returns a UILabel containing the hint for the omnibox, added to the
On 2017/05/10 09:11:26, marq (ping after 24h) wrote:
> On 2017/05/03 15:27:35, gambard wrote:
> > On 2017/05/03 15:22:05, marq (ping after 24h) wrote:
> > > Normally I would object to a function that does this kind of thing, but
> since
> > > this is in the context of major refactoring, I'm fine with it for the time
> > > being.
> > 
> > Which part are you disagreeing on? My first intent was to create the object
> and
> > add it to the searchTapTarget outside this helper, then only configure it
> > (including the constraint) in the helper. Would that be more acceptable?
> 
> Yes; in general I don't like functions that return values and also have side
> effects, so having a utility that just arranges existing views passed into it
> seems cleaner. 

Acknowledged.

https://codereview.chromium.org/2855113003/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_utils.h:35:
// Returns a button for the voice search, added to the |searcgTapTarget| and
On 2017/05/03 15:22:05, marq (ping after 24h) wrote:
> searcg -> search

Done.

[gambard, 2017-05-10 13:14:26]

The CQ bit was checked by gambard@chromium.org

[gambard, 2017-05-10 13:14:27]

The patchset sent to the CQ was uploaded after l-g-t-m from marq@chromium.org
Link to the patchset: https://codereview.chromium.org/2855113003/#ps40001
(title: "Address comments")

[commit-bot: I haz the power, 2017-05-10 13:14:43]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 15:41:57]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494422066997860,
"parent_rev": "de3013b3e1ca504de8784c13b66f45dc52cab0ae", "commit_rev":
"02b01701047edf186f7d4164af8bcc6a0c78594f"}

[commit-bot: I haz the power, 2017-05-10 15:42:12]

Description was changed from

==========
Move the fake omnibox constraints out of GoogleLandingViewController

In order to refactor GoogleLandingViewController, the configuration of the
elements displayed in the fake omnibox are moved to a helper.

BUG=717974
==========

to

==========
Move the fake omnibox constraints out of GoogleLandingViewController

In order to refactor GoogleLandingViewController, the configuration of the
elements displayed in the fake omnibox are moved to a helper.

BUG=717974

Review-Url: https://codereview.chromium.org/2855113003
Cr-Commit-Position: refs/heads/master@{#470583}
Committed:
https://chromium.googlesource.com/chromium/src/+/02b01701047edf186f7d4164af8b...
==========

[commit-bot: I haz the power, 2017-05-10 15:42:13]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/02b01701047edf186f7d4164af8b...