AppLauncher Touch Feedback

AppLauncher Touch Feedback

Add visual feedback to AppListButton. The background will draw active in response to touch downs.

TEST=ShelfViewTest.AppListButtonTouchFeedback, ShelfViewTest.AppListButtonTouchFeedbackCancellation
BUG=398398
Committed: https://crrev.com/26c4cb3ebb2012e2afe751f97dcc1085ded1ca96
Cr-Commit-Position: refs/heads/master@{#294814}

[jonross, 2014-09-09 17:42:22]

jonross@chromium.org changed reviewers:
+ flackr@chromium.org

[jonross, 2014-09-09 17:42:22]

Hi Rob,

Another touch feedback implementation.

[flackr, 2014-09-10 20:04:42]

https://codereview.chromium.org/558703002/diff/1/ash/shelf/app_list_button.cc
File ash/shelf/app_list_button.cc (right):

https://codereview.chromium.org/558703002/diff/1/ash/shelf/app_list_button.cc...
ash/shelf/app_list_button.cc:190: if (!touch_feedback_enabled_)
nit: A little confusing that calling SetDrawBackgroundaAsActive will do nothing
unless the flag is passed. Maybe ShowTouchFeedbackTouched or something to imply
that this is specifically for touch feedback and not to just set the active
state.

Also, this probably isn't called too frequently, i.e. could just check the flag.

https://codereview.chromium.org/558703002/diff/1/ash/shelf/app_list_button.h
File ash/shelf/app_list_button.h (right):

https://codereview.chromium.org/558703002/diff/1/ash/shelf/app_list_button.h#...
ash/shelf/app_list_button.h:50: bool set_draw_background_as_active_;
nit: s/set_draw_background_as_active_/draw_background_as_active_

[jonross, 2014-09-11 15:06:52]

https://codereview.chromium.org/558703002/diff/1/ash/shelf/app_list_button.cc
File ash/shelf/app_list_button.cc (right):

https://codereview.chromium.org/558703002/diff/1/ash/shelf/app_list_button.cc...
ash/shelf/app_list_button.cc:190: if (!touch_feedback_enabled_)
On 2014/09/10 20:04:41, flackr wrote:
> nit: A little confusing that calling SetDrawBackgroundaAsActive will do
nothing
> unless the flag is passed. Maybe ShowTouchFeedbackTouched or something to
imply
> that this is specifically for touch feedback and not to just set the active
> state.
> 
> Also, this probably isn't called too frequently, i.e. could just check the
flag.

Since we want to eventually eliminate the flag I've moved this check to the
calls to SetDrawBackgroundAsActive, rather than renaming the method.

https://codereview.chromium.org/558703002/diff/1/ash/shelf/app_list_button.h
File ash/shelf/app_list_button.h (right):

https://codereview.chromium.org/558703002/diff/1/ash/shelf/app_list_button.h#...
ash/shelf/app_list_button.h:50: bool set_draw_background_as_active_;
On 2014/09/10 20:04:41, flackr wrote:
> nit: s/set_draw_background_as_active_/draw_background_as_active_

Done.

[flackr, 2014-09-11 21:01:02]

lgtm with nit

https://codereview.chromium.org/558703002/diff/20001/ash/shelf/app_list_butto...
File ash/shelf/app_list_button.cc (right):

https://codereview.chromium.org/558703002/diff/20001/ash/shelf/app_list_butto...
ash/shelf/app_list_button.cc:50: touch_feedback_enabled_ = true;
initializer list.

[jonross, 2014-09-12 17:16:08]

jonross@chromium.org changed reviewers:
+ skuhne@google.com

[jonross, 2014-09-12 17:16:09]

Hi Stefan,

In this review I am adding touch feedback to the application launcher.

https://codereview.chromium.org/558703002/diff/20001/ash/shelf/app_list_butto...
File ash/shelf/app_list_button.cc (right):

https://codereview.chromium.org/558703002/diff/20001/ash/shelf/app_list_butto...
ash/shelf/app_list_button.cc:50: touch_feedback_enabled_ = true;
On 2014/09/11 21:01:02, flackr wrote:
> initializer list.

Done.

[jonross, 2014-09-12 17:16:28]

jonross@chromium.org changed reviewers:
+ skuhne@chromium.org
- skuhne@google.com

[Mr4D (OOO till 08-26), 2014-09-12 23:23:26]

lgtm

[jonross, 2014-09-15 12:52:26]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2014-09-15 12:52:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/558703002/40001

[commit-bot: I haz the power, 2014-09-15 13:50:58]

Committed patchset #3 (id:40001) as da7196ac06c4687e0f0b563b35f3952951561537

[commit-bot: I haz the power, 2014-09-15 15:08:25]

Patchset 3 (id:??) landed as
https://crrev.com/26c4cb3ebb2012e2afe751f97dcc1085ded1ca96
Cr-Commit-Position: refs/heads/master@{#294814}