Use a background for the main search box in the experimental app list.

Use a background for the main search box in the experimental app list.

This CL makes the DummySearchBoxBackground the standard SearchBoxView
background when using the experimental app list.

BUG=406222
Committed: https://crrev.com/9f4e92092dc37cfd40044d8686612c771b33f168
Cr-Commit-Position: refs/heads/master@{#291855}

[Matt Giuca, 2014-08-22 03:52:47]

Can you post before+after screenshots on the bug?

[calamity, 2014-08-25 03:49:43]

On 2014/08/22 03:52:47, Matt Giuca wrote:
> Can you post before+after screenshots on the bug?

Done.

[Matt Giuca, 2014-08-25 05:15:25]

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/app_li...
File ui/app_list/views/app_list_main_view.cc (right):

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/app_li...
ui/app_list/views/app_list_main_view.cc:46: const int
kExperimentalSearchBoxViewPadding = 24;
Don't use "Experimental" here. It won't be experimental forever so I think we
can start using vanilla language unless there is a conflict.

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/search...
File ui/app_list/views/search_box_view.cc (right):

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/search...
ui/app_list/views/search_box_view.cc:53: class ExperimentalBackground : public
views::Background {
Same here: Call this BorderedBackground or similar.

(Feel free to say in the comments "used by the experimental app list.")

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/start_...
File ui/app_list/views/start_page_view.cc (right):

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/start_...
ui/app_list/views/start_page_view.cc:57: size.set_width(kDummySearchBoxWidth);
Looks like there's literally just the width of the search box is the only
difference between DummySearchBoxView and its parent class.

I don't think it's worth having this as a subclass any more. Can we just
override the width using a parameter to SearchBoxView or something like that?

[calamity, 2014-08-26 01:16:30]

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/app_li...
File ui/app_list/views/app_list_main_view.cc (right):

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/app_li...
ui/app_list/views/app_list_main_view.cc:46: const int
kExperimentalSearchBoxViewPadding = 24;
On 2014/08/25 05:15:25, Matt Giuca wrote:
> Don't use "Experimental" here. It won't be experimental forever so I think we
> can start using vanilla language unless there is a conflict.

Done.

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/search...
File ui/app_list/views/search_box_view.cc (right):

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/search...
ui/app_list/views/search_box_view.cc:53: class ExperimentalBackground : public
views::Background {
On 2014/08/25 05:15:25, Matt Giuca wrote:
> Same here: Call this BorderedBackground or similar.
> 
> (Feel free to say in the comments "used by the experimental app list.")

Done. I don't think the comment will be necessary..

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/start_...
File ui/app_list/views/start_page_view.cc (right):

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/start_...
ui/app_list/views/start_page_view.cc:57: size.set_width(kDummySearchBoxWidth);
On 2014/08/25 05:15:25, Matt Giuca wrote:
> Looks like there's literally just the width of the search box is the only
> difference between DummySearchBoxView and its parent class.
> 
> I don't think it's worth having this as a subclass any more. Can we just
> override the width using a parameter to SearchBoxView or something like that?

I _could_ but I'm planning to remove this completely and just use 1 search box
at some point. That will be necessary for the search box animation so I think we
can just leave it for now.

[Matt Giuca, 2014-08-26 02:45:33]

lgtm

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/start_...
File ui/app_list/views/start_page_view.cc (right):

https://codereview.chromium.org/493293003/diff/40001/ui/app_list/views/start_...
ui/app_list/views/start_page_view.cc:57: size.set_width(kDummySearchBoxWidth);
On 2014/08/26 01:16:30, calamity wrote:
> On 2014/08/25 05:15:25, Matt Giuca wrote:
> > Looks like there's literally just the width of the search box is the only
> > difference between DummySearchBoxView and its parent class.
> > 
> > I don't think it's worth having this as a subclass any more. Can we just
> > override the width using a parameter to SearchBoxView or something like
that?
> 
> I _could_ but I'm planning to remove this completely and just use 1 search box
> at some point. That will be necessary for the search box animation so I think
we
> can just leave it for now.

Acknowledged.

[calamity, 2014-08-26 03:26:50]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2014-08-26 03:28:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/calamity@chromium.org/493293003/60001

[commit-bot: I haz the power, 2014-08-26 06:30:30]

Committed patchset #3 (60001) as e72030038682d74a413cfbc690658405b62b0dfe

[commit-bot: I haz the power, 2014-09-10 02:41:00]

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