New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 661380 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Search in Launcher shows empty apps box when no apps results found

Project Member Reported by kuscher@chromium.org, Nov 2 2016

Issue description

Chrome Version       : 54.0.2840.79
OS Version: 8743.76.0

Repro

1) Open Launcher
2) Type search query into searchbox
3) Keep typing until there are no app matches

Expected
When no matches are found, do not show apps result box

Actual
It shows an empty apps box at the bottom. See screenshot
 
Screenshot 2016-11-01 at 17.21.12.png
79.3 KB View Download
Cc: -glevin@chromium.org abodenha@chromium.org
Owner: glevin@chromium.org
Status: Started (was: Assigned)
Cc: mtomasz@chromium.org
Bisect:
You are probably looking for a change made after 407077 (known good), but no later than 407078 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/d8a924a2dd4693f40a27b33efc2de3db46a2496e..e9b526245329d9f5106ddcf9c7791a17b2d0650b

Looks like it was https://codereview.chromium.org/2165483002.

The problem was switching from BoxLayout to GridLayout, at least as implemented.  The BoxLayout would hide itself when empty; the GridLayout doesn't.
In the aforementioned CL, the Launcher's app search results box was switched from using BoxLayout to GridLayout.  This was apparently for aesthetic reasons, but things seem fine now with the old BoxLayout.  I'm attaching screenshots of the box with both layout managers (BoxLayout from my proposed CL (https://codereview.chromium.org/2473033002) and GridLayout from the current code).  The icon placement is identical; the only difference is space allowed for app names, and I'd say that BoxLayout is actually better as the text doesn't go right to the edge of the box.  I tried this with 2, 4, 7, and 8 results, and in all cases, icon placement was identical.
7BoxLayout_2px.png
24.2 KB View Download
7GridLayout.png
23.8 KB View Download
Looks good, but how about selection? Did it change? If yes, then could you provide a screenshot? Thanks!

Comment 5 by glevin@chromium.org, Nov 16 2016

Selection behaves identically, as far as I can tell, and looks *almost* the same.  I've attached screenshots of Box and Grid layouts, with one item selected, and the next hover-highlighted.  When BoxLayout is used, there is 1px of whitespace between selection box and edge of container, and 2px of whitespace between selection and hover boxes; when GridLayout is used, there is no whitespace.

The 1 or 2px whitespace padding maybe looks a little worse, but I think that's offset by the text not running right up to the edge.  A tossup in my opinion.
8BoxLayout_2px.png
25.5 KB View Download
8GridLayout.png
25.5 KB View Download
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32247f5271fd583a683bc8dc2e4c07ce3cf72b85

commit 32247f5271fd583a683bc8dc2e4c07ce3cf72b85
Author: glevin <glevin@chromium.org>
Date: Mon Nov 21 23:19:06 2016

Hide Launcher app search box when empty

BUG= 661380 
TEST=Open launcher, type a search that returns no app results.  The
horizontal box that usually contains app results should be hidden.

Review-Url: https://codereview.chromium.org/2473033002
Cr-Commit-Position: refs/heads/master@{#433686}

[modify] https://crrev.com/32247f5271fd583a683bc8dc2e4c07ce3cf72b85/ui/app_list/views/search_result_tile_item_list_view.cc

Comment 7 by glevin@chromium.org, Nov 21 2016

Labels: Merge-Request-56 Merge-Request-55

Comment 8 by dimu@chromium.org, Nov 22 2016

Labels: -Merge-Request-55 Merge-Review-55 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M55, manual review required.

Comment 9 by dimu@chromium.org, Nov 22 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Labels: -Hotlist-Merge-Review -Merge-Review-55 Merge-Approved-55
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 29 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/956718c5d2933fdc7cd6df90339565aa647e3213

commit 956718c5d2933fdc7cd6df90339565aa647e3213
Author: glevin <glevin@chromium.org>
Date: Mon Nov 28 23:56:31 2016

Hide Launcher app search box when empty

BUG= 661380 
TEST=Open launcher, type a search that returns no app results.  The
horizontal box that usually contains app results should be hidden.

Review-Url: https://codereview.chromium.org/2473033002
Cr-Commit-Position: refs/heads/master@{#433686}
(cherry picked from commit 32247f5271fd583a683bc8dc2e4c07ce3cf72b85)

Review URL: https://codereview.chromium.org/2534043002 .

Cr-Commit-Position: refs/branch-heads/2883@{#672}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/956718c5d2933fdc7cd6df90339565aa647e3213/ui/app_list/views/search_result_tile_item_list_view.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 29 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd7b6ddecbf986fa18f8f6290c7ab700cbc6e5a1

commit dd7b6ddecbf986fa18f8f6290c7ab700cbc6e5a1
Author: glevin <glevin@chromium.org>
Date: Tue Nov 29 00:26:52 2016

Hide Launcher app search box when empty

BUG= 661380 
TEST=Open launcher, type a search that returns no app results.  The
horizontal box that usually contains app results should be hidden.

Review-Url: https://codereview.chromium.org/2473033002
Cr-Commit-Position: refs/heads/master@{#433686}
(cherry picked from commit 32247f5271fd583a683bc8dc2e4c07ce3cf72b85)

Review URL: https://codereview.chromium.org/2536053002 .

Cr-Commit-Position: refs/branch-heads/2924@{#144}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/dd7b6ddecbf986fa18f8f6290c7ab700cbc6e5a1/ui/app_list/views/search_result_tile_item_list_view.cc

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
Verified on ChromeOS 8872.65.0, 55.0.2883.76

Sign in to add a comment