cros: add SUGGESTED APPS indicator to start page

cros: add SUGGESTED APPS indicator to start page

Changes:
(1) Create IndicatorChipView, as it is needed for both suggested apps
and all apps.
(2) add 24px top padding for search box on start page.
(3) change start page search box width to 544px.
(4) 21px padding between search box and indicator.

WIP screenshot: https://screenshot.googleplex.com/XqW9ZXqVVnO
Note: tiles layout are not updated yet.

R=xiyuan@chromium.org
CC=newcomer@chromium.org
BUG=734164
TEST=tested with and without --enable-features=EnableFullscreenAppList
flag

Review-Url: https://codereview.chromium.org/2952303002
Cr-Commit-Position: refs/heads/master@{#481745}
Committed: https://chromium.googlesource.com/chromium/src/+/4eb7d7b4449fee720181f73e4ea0947fc903b88d

[Qiang(Joe) Xu, 2017-06-22 21:43:09]

Description was changed from

==========
cros: add SUGGESTED APPS indicator to start page

Changes:
(1) Create IndicatorChipView, as it is needed for both suggested apps
and all apps.
(2) add 24px top padding for search box on start page.
(3) change start page search box width to 544px.
(4) 21px padding between search box and indicator.

R=xiyuan@chromium.org
CC=newcomer@chromium.org
BUG=734164
TEST=
==========

to

==========
cros: add SUGGESTED APPS indicator to start page

Changes:
(1) Create IndicatorChipView, as it is needed for both suggested apps
and all apps.
(2) add 24px top padding for search box on start page.
(3) change start page search box width to 544px.
(4) 21px padding between search box and indicator.

WIP screenshot: https://screenshot.googleplex.com/XqW9ZXqVVnO
Note: tiles layout are not updated yet.

R=xiyuan@chromium.org
CC=newcomer@chromium.org
BUG=734164
TEST=tested with and without --enable-features=EnableFullscreenAppList
flag
==========

[Qiang(Joe) Xu, 2017-06-22 21:46:49]

Hi xiyuan, PTAL thanks.

Note, IDS_ALL_APPS_INDICATOR is not used in this CL. But string freeze is
tomorrow. Could we check in this together?

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/start_pag...
File ui/app_list/views/start_page_view.h (left):

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/start_pag...
ui/app_list/views/start_page_view.h:79: StartPageTilesContainer*
tiles_container_;  // Owned by views hierarchy.
We might want to move StartPageTilesContainer out of StartPageView,
as for new design, it is needed on both start page and the first page
of all apps.

We can rename it something like SuggestedAppTilesContainer.

[xiyuan, 2017-06-22 22:33:52]

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
File ui/app_list/views/indicator_chip_view.cc (right):

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
ui/app_list/views/indicator_chip_view.cc:55: gfx::FontList small_font =
gfx::FontList -> const gfx::FontList&

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
ui/app_list/views/indicator_chip_view.cc:59:
label_->SetBorder(views::NullBorder());
border is null by default. We probably can remove this.

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
ui/app_list/views/indicator_chip_view.cc:62:
label_->SetBackgroundColor(SK_ColorTRANSPARENT);
Try to avoid SK_ColorTRANSPARENT background. Font anti-alias and hinting might
not work properly on a transparent background. If |label_| is always inside
|container_|, we can use kBackgroundColor for label background.

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
File ui/app_list/views/indicator_chip_view.h (right):

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
ui/app_list/views/indicator_chip_view.h:21: explicit
IndicatorChipView(base::string16 text);
nit: base::string16 -> const base::string16&

and #include "base/strings/string16.h"

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/start_pag...
File ui/app_list/views/start_page_view.cc (right):

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/start_pag...
ui/app_list/views/start_page_view.cc:290: indicator_(nullptr),
nit: initialize in header

[Qiang(Joe) Xu, 2017-06-22 22:47:27]

feedback addressed, thanks

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
File ui/app_list/views/indicator_chip_view.cc (right):

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
ui/app_list/views/indicator_chip_view.cc:55: gfx::FontList small_font =
On 2017/06/22 22:33:51, xiyuan wrote:
> gfx::FontList -> const gfx::FontList&

Done.

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
ui/app_list/views/indicator_chip_view.cc:59:
label_->SetBorder(views::NullBorder());
On 2017/06/22 22:33:52, xiyuan wrote:
> border is null by default. We probably can remove this.

Done.

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
ui/app_list/views/indicator_chip_view.cc:62:
label_->SetBackgroundColor(SK_ColorTRANSPARENT);
On 2017/06/22 22:33:52, xiyuan wrote:
> Try to avoid SK_ColorTRANSPARENT background. Font anti-alias and hinting might
> not work properly on a transparent background. If |label_| is always inside
> |container_|, we can use kBackgroundColor for label background.

acknowledged, done

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
File ui/app_list/views/indicator_chip_view.h (right):

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/indicator...
ui/app_list/views/indicator_chip_view.h:21: explicit
IndicatorChipView(base::string16 text);
On 2017/06/22 22:33:52, xiyuan wrote:
> nit: base::string16 -> const base::string16&
> 
> and #include "base/strings/string16.h"

Done.

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/start_pag...
File ui/app_list/views/start_page_view.cc (right):

https://codereview.chromium.org/2952303002/diff/1/ui/app_list/views/start_pag...
ui/app_list/views/start_page_view.cc:290: indicator_(nullptr),
On 2017/06/22 22:33:52, xiyuan wrote:
> nit: initialize in header

Done.

[xiyuan, 2017-06-22 22:48:39]

lgtm

[Qiang(Joe) Xu, 2017-06-22 22:50:26]

The CQ bit was checked by warx@chromium.org

[commit-bot: I haz the power, 2017-06-22 22:50:58]

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-06-23 00:29:34]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498171826083770,
"parent_rev": "2fad1a80b82ed7df3a3967192a07d8f55e1bab0a", "commit_rev":
"4eb7d7b4449fee720181f73e4ea0947fc903b88d"}

[commit-bot: I haz the power, 2017-06-23 00:29:46]

Description was changed from

==========
cros: add SUGGESTED APPS indicator to start page

Changes:
(1) Create IndicatorChipView, as it is needed for both suggested apps
and all apps.
(2) add 24px top padding for search box on start page.
(3) change start page search box width to 544px.
(4) 21px padding between search box and indicator.

WIP screenshot: https://screenshot.googleplex.com/XqW9ZXqVVnO
Note: tiles layout are not updated yet.

R=xiyuan@chromium.org
CC=newcomer@chromium.org
BUG=734164
TEST=tested with and without --enable-features=EnableFullscreenAppList
flag
==========

to

==========
cros: add SUGGESTED APPS indicator to start page

Changes:
(1) Create IndicatorChipView, as it is needed for both suggested apps
and all apps.
(2) add 24px top padding for search box on start page.
(3) change start page search box width to 544px.
(4) 21px padding between search box and indicator.

WIP screenshot: https://screenshot.googleplex.com/XqW9ZXqVVnO
Note: tiles layout are not updated yet.

R=xiyuan@chromium.org
CC=newcomer@chromium.org
BUG=734164
TEST=tested with and without --enable-features=EnableFullscreenAppList
flag

Review-Url: https://codereview.chromium.org/2952303002
Cr-Commit-Position: refs/heads/master@{#481745}
Committed:
https://chromium.googlesource.com/chromium/src/+/4eb7d7b4449fee720181f73e4ea0...
==========

[commit-bot: I haz the power, 2017-06-23 00:29:50]

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