[Home] Placeholder sheet content for omnibox focus

[Home] Placeholder sheet content for omnibox focus

When the omnibox is being focused from the sheet's peeking state, the
bottom sheet content will be hidden to avoid excessive visual noise.
When the omnibox loses focus, the content is replaced by the
suggestions content. If the sheet is already expanded when the omnibox
is focused, the content will remain visible.

This change also re-enables zero-suggest for Chrome Home.

BUG=716224
Review-Url: https://codereview.chromium.org/2876093002
Cr-Commit-Position: refs/heads/master@{#471372}
Committed: https://chromium.googlesource.com/chromium/src/+/b9fc5a9b20adc654cbf6021ba20d6d5931b6f519

[mdjones, 2017-05-12 01:33:44]

Description was changed from

==========
[Home] Placeholder sheet content for omnibox focus

When the omnibox is being focused from the sheet's peeking state, the
bottom sheet content will be hidden to avoid excessive visual noise.
When the omnibox loses focus, the content is replaced by the
suggestions content. If the sheet is already expanded when the omnibox
is focused, the content will remain visible.

BUG=716224
==========

to

==========
[Home] Placeholder sheet content for omnibox focus

When the omnibox is being focused from the sheet's peeking state, the
bottom sheet content will be hidden to avoid excessive visual noise.
When the omnibox loses focus, the content is replaced by the
suggestions content. If the sheet is already expanded when the omnibox
is focused, the content will remain visible.

This change also re-enables zero-suggest for Chrome Home.

BUG=716224
==========

[mdjones, 2017-05-12 01:37:02]

mdjones@chromium.org changed reviewers:
+ twellington@chromium.org

[mdjones, 2017-05-12 01:37:03]

Once the bottom nav is visible, this may require some extra thought. ptal

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetContentController.java
(right):

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetContentController.java:72:
private static final int PLACEHOLDER_ID = -2;
Alternatively, I can have a "private final BottomSheetContent" that we can hold
on to. I don't have strong feelings either way.

[Theresa, 2017-05-12 15:15:55]

lgtm

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheet.java
(right):

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheet.java:394:
mContentSwapAnimatorSet.end();
Should we set mContentSwapAnimatorSet = null here?

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetContentController.java
(right):

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetContentController.java:72:
private static final int PLACEHOLDER_ID = -2;
On 2017/05/12 01:37:03, mdjones wrote:
> Alternatively, I can have a "private final BottomSheetContent" that we can
hold
> on to. I don't have strong feelings either way.

I think this is fine. We can revisit when we look at when to destroy
SuggestionsBottomSheetContent.

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetContentController.java:217:
// If the omnibox is not being focused, show the default suggestions content.
This comment seems like it goes better with the if (!hasFocus...) block.

[mdjones, 2017-05-12 16:07:22]

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheet.java
(right):

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheet.java:394:
mContentSwapAnimatorSet.end();
On 2017/05/12 15:15:55, Theresa wrote:
> Should we set mContentSwapAnimatorSet = null here?

Good catch, done.

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetContentController.java
(right):

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetContentController.java:72:
private static final int PLACEHOLDER_ID = -2;
On 2017/05/12 15:15:55, Theresa wrote:
> On 2017/05/12 01:37:03, mdjones wrote:
> > Alternatively, I can have a "private final BottomSheetContent" that we can
> hold
> > on to. I don't have strong feelings either way.
> 
> I think this is fine. We can revisit when we look at when to destroy
> SuggestionsBottomSheetContent.

Acknowledged.

https://codereview.chromium.org/2876093002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetContentController.java:217:
// If the omnibox is not being focused, show the default suggestions content.
On 2017/05/12 15:15:55, Theresa wrote:
> This comment seems like it goes better with the if (!hasFocus...) block.

Done.

[mdjones, 2017-05-12 16:08:18]

mdjones@chromium.org changed reviewers:
+ tedchoc@chromium.org

[mdjones, 2017-05-12 16:08:19]

+tedchoc for owners

[Ted C, 2017-05-12 16:37:32]

lgtm

[mdjones, 2017-05-12 16:42:47]

The CQ bit was checked by mdjones@chromium.org

[mdjones, 2017-05-12 16:42:48]

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

[commit-bot: I haz the power, 2017-05-12 16:43:19]

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-12 18:22:57]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494607367885790,
"parent_rev": "80e847abe926081b3e2e9b0f249548be84d4ac14", "commit_rev":
"b9fc5a9b20adc654cbf6021ba20d6d5931b6f519"}

[commit-bot: I haz the power, 2017-05-12 18:23:10]

Description was changed from

==========
[Home] Placeholder sheet content for omnibox focus

When the omnibox is being focused from the sheet's peeking state, the
bottom sheet content will be hidden to avoid excessive visual noise.
When the omnibox loses focus, the content is replaced by the
suggestions content. If the sheet is already expanded when the omnibox
is focused, the content will remain visible.

This change also re-enables zero-suggest for Chrome Home.

BUG=716224
==========

to

==========
[Home] Placeholder sheet content for omnibox focus

When the omnibox is being focused from the sheet's peeking state, the
bottom sheet content will be hidden to avoid excessive visual noise.
When the omnibox loses focus, the content is replaced by the
suggestions content. If the sheet is already expanded when the omnibox
is focused, the content will remain visible.

This change also re-enables zero-suggest for Chrome Home.

BUG=716224

Review-Url: https://codereview.chromium.org/2876093002
Cr-Commit-Position: refs/heads/master@{#471372}
Committed:
https://chromium.googlesource.com/chromium/src/+/b9fc5a9b20adc654cbf6021ba20d...
==========

[commit-bot: I haz the power, 2017-05-12 18:23:12]

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