🏠 Allow bottom sheet content to update handle color

[Home] Allow bottom sheet content to update handle color

This change adds some plumbing to allow the toolbars of the bottom
sheet to update the handle color based on their state. The
BottomSheetContent interface has been updated to allow the bottom
sheet to query the current handle color if a custom toolbar is
showing and each toolbar has a reference to the bottom sheet to
request an update.

BUG=712689
Review-Url: https://codereview.chromium.org/2833573002
Cr-Commit-Position: refs/heads/master@{#466164}
Committed: https://chromium.googlesource.com/chromium/src/+/3de25d5d2440ec1df7c04ec0b3fa639450bd6105

[mdjones, 2017-04-19 20:15:42]

Description was changed from

==========
[Home] Allow bottom sheet content to update handle color

This change adds some plumbing to allow the toolbars of the bottom
sheet to update the handle color based on their state. The
BottomSheetContent interface has been updated to allow the bottom
sheet to query the current handle color if a custom toolbar is
showing and each toolbar has a reference to the bottom sheet to
request an update.

BUG=712689
==========

to

==========
[Home] Allow bottom sheet content to update handle color

This change adds some plumbing to allow the toolbars of the bottom
sheet to update the handle color based on their state. The
BottomSheetContent interface has been updated to allow the bottom
sheet to query the current handle color if a custom toolbar is
showing and each toolbar has a reference to the bottom sheet to
request an update.

BUG=712689
==========

[mdjones, 2017-04-19 20:17:56]

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

[mdjones, 2017-04-19 20:17:57]

There are some other options to this implementation that don't necessarily
involve the sheet content knowing about the specific kind of toolbar, ptal.

[Theresa, 2017-04-19 21:28:29]

I would prefer the selection widgets not to have a hard dependency on
BottomSheet. Rather, I'd like BookmarkSheetContent, DownloadSheetContent, and
HistorySheetContent own the connection with the bottom sheet.

What if the SelectableListToolbar had an observer interface (with unfortunately
only one method for now), like:

Observer {
  void onToolbarColorChanged(boolean isBackgroundLightColor);
}

*SheetContent could add an observer to the toolbar and call into the BottomSheet
to update the tint when it does.

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

https://codereview.chromium.org/2833573002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheet.java:577:
mDefaultToolbarView.updateHandleTint(isLight);
I'm interpreting isUsingLightToolbar() to mean that the toolbar itself is light
(e.g. grey or white), which would be we should use a dark handle.

Based on the method names, I would expect:
isUsingLightToolbar() -> updateHandleTint(useLightHandle = false)

[mdjones, 2017-04-19 22:49:45]

Added observers so BottomSheet isn't being passed everywhere.

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

https://codereview.chromium.org/2833573002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheet.java:577:
mDefaultToolbarView.updateHandleTint(isLight);
On 2017/04/19 21:28:29, Theresa wrote:
> I'm interpreting isUsingLightToolbar() to mean that the toolbar itself is
light
> (e.g. grey or white), which would be we should use a dark handle.
> 
> Based on the method names, I would expect:
> isUsingLightToolbar() -> updateHandleTint(useLightHandle = false)

Yeah, I some of the names got confised. Fixed.

[Theresa, 2017-04-19 23:48:48]

lgtm

https://codereview.chromium.org/2833573002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSheetContent.java
(right):

https://codereview.chromium.org/2833573002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSheetContent.java:81:
public boolean isUsingLightToolbar() {
nit: should this be isUsingLightToolbarTheme() for consistency?

https://codereview.chromium.org/2833573002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectableListToolbar.java
(right):

https://codereview.chromium.org/2833573002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectableListToolbar.java:79:
*                     parameter is true, it indicated that dark drawables should
be used.
nit: s/indicated/indicates

[mdjones, 2017-04-20 00:03:33]

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

[mdjones, 2017-04-20 00:03:33]

+dfalcantara owners

https://codereview.chromium.org/2833573002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSheetContent.java
(right):

https://codereview.chromium.org/2833573002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSheetContent.java:81:
public boolean isUsingLightToolbar() {
On 2017/04/19 23:48:47, Theresa wrote:
> nit: should this be isUsingLightToolbarTheme() for consistency?

Yep, done.

https://codereview.chromium.org/2833573002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectableListToolbar.java
(right):

https://codereview.chromium.org/2833573002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectableListToolbar.java:79:
*                     parameter is true, it indicated that dark drawables should
be used.
On 2017/04/19 23:48:47, Theresa wrote:
> nit: s/indicated/indicates

Done.

[gone, 2017-04-20 20:58:45]

rs lgtm

[mdjones, 2017-04-20 21:38:01]

The CQ bit was checked by mdjones@chromium.org

[mdjones, 2017-04-20 21:38:01]

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/2833573002/#ps40001
(title: "nits")

[commit-bot: I haz the power, 2017-04-20 21:38:30]

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-04-20 22:28:27]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492724281396660,
"parent_rev": "0c12d093e94c924034f682abe7e7f643453fd28c", "commit_rev":
"3de25d5d2440ec1df7c04ec0b3fa639450bd6105"}

[commit-bot: I haz the power, 2017-04-20 22:28:40]

Description was changed from

==========
[Home] Allow bottom sheet content to update handle color

This change adds some plumbing to allow the toolbars of the bottom
sheet to update the handle color based on their state. The
BottomSheetContent interface has been updated to allow the bottom
sheet to query the current handle color if a custom toolbar is
showing and each toolbar has a reference to the bottom sheet to
request an update.

BUG=712689
==========

to

==========
[Home] Allow bottom sheet content to update handle color

This change adds some plumbing to allow the toolbars of the bottom
sheet to update the handle color based on their state. The
BottomSheetContent interface has been updated to allow the bottom
sheet to query the current handle color if a custom toolbar is
showing and each toolbar has a reference to the bottom sheet to
request an update.

BUG=712689

Review-Url: https://codereview.chromium.org/2833573002
Cr-Commit-Position: refs/heads/master@{#466164}
Committed:
https://chromium.googlesource.com/chromium/src/+/3de25d5d2440ec1df7c04ec0b3fa...
==========

[commit-bot: I haz the power, 2017-04-20 22:28:41]

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