New issue
Advanced search Search tips

Issue 630383 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 608535



Sign in to add a comment

MD Settings: Implement "No results" message for search.

Project Member Reported by dpa...@chromium.org, Jul 21 2016

Issue description

When a search produces no results a "No matches were found" message should be displayed to the user (see [1] and more detailed mocks at [2]).

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=602469#c20
[2] https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Inner%20Pages/03-Settings/specs#%2FSPEC-settings_searchUI.png%3Fz=width
 
Labels: Hotlist-MD-Settings-SearchBox

Comment 2 by dpa...@chromium.org, Jul 23 2016

Status: Started (was: Assigned)
This is broken in two tasks basically.
1) Expose whether any results were found during search. Currently
   the searching algorithm does not expose such information.
2) Update the UI based on the previously exposed information.

Started on 1.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 26 2016

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

commit 3cc76ff035eca6a47c8dfe31883a4b9978ee2432
Author: dpapad <dpapad@chromium.org>
Date: Tue Jul 26 18:31:05 2016

MD Settings: Implementing a "no results" page.

Specifically:
 - Removing mechanism of observing SearchManager for now.
 - Replacing with a Promise that is returned from SearchManager#search().
 - Renaming "SearchContext" class to "SearchRequest" and resolving search()
   promises with SearchRequest objects.
 - Updating <settings-main> to display a "no results" page when no results
   are found.

BUG= 630383 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/3cc76ff035eca6a47c8dfe31883a4b9978ee2432/chrome/app/settings_strings.grdp
[modify] https://crrev.com/3cc76ff035eca6a47c8dfe31883a4b9978ee2432/chrome/browser/resources/settings/search_settings.js
[modify] https://crrev.com/3cc76ff035eca6a47c8dfe31883a4b9978ee2432/chrome/browser/resources/settings/settings_main/settings_main.html
[modify] https://crrev.com/3cc76ff035eca6a47c8dfe31883a4b9978ee2432/chrome/browser/resources/settings/settings_main/settings_main.js
[modify] https://crrev.com/3cc76ff035eca6a47c8dfe31883a4b9978ee2432/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 3 2016

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

commit bb8fba2c2dbc0b02555f11877f4929786eab456c
Author: dpapad <dpapad@chromium.org>
Date: Wed Aug 03 20:42:17 2016

MD Settings: Adding some unit tests for <settings-main>.

Specifically testing the "no results" message is shown/hidden as expected.
 - Splitting SearchManager to an interface and an implementation.
 - Using a TestSearchManager class (implements SearchManager) for testing
   <settings-main>.

BUG= 630383 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/bb8fba2c2dbc0b02555f11877f4929786eab456c/chrome/browser/resources/settings/search_settings.js
[modify] https://crrev.com/bb8fba2c2dbc0b02555f11877f4929786eab456c/chrome/browser/resources/settings/settings_main/settings_main.html
[modify] https://crrev.com/bb8fba2c2dbc0b02555f11877f4929786eab456c/chrome/browser/resources/settings/settings_main/settings_main.js
[modify] https://crrev.com/bb8fba2c2dbc0b02555f11877f4929786eab456c/chrome/browser/resources/settings/settings_page/main_page_behavior.js
[modify] https://crrev.com/bb8fba2c2dbc0b02555f11877f4929786eab456c/chrome/test/data/webui/settings/cr_settings_browsertest.js
[add] https://crrev.com/bb8fba2c2dbc0b02555f11877f4929786eab456c/chrome/test/data/webui/settings/settings_main_test.js

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 4 2016

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

commit 563a4d1a4a02714746f0782cafea9c5ad247c4ae
Author: peter <peter@chromium.org>
Date: Thu Aug 04 15:28:21 2016

Revert of MD Settings: Adding some unit tests for <settings-main>. (patchset #4 id:360001 of https://codereview.chromium.org/2185493003/ )

Reason for revert:
Causing various debug bots to fail browser_tests

https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/31788/steps/browser_tests%20on%20Ubuntu-12.04/logs/CrSettingsMainPageTest.All

https://build.chromium.org/p/chromium.mac/builders/Mac10.9%20Tests%20%28dbg%29/builds/28095/steps/browser_tests%20on%20Mac-10.9/logs/CrSettingsMainPageTest.All

A series of JavaScript errors are visible that preferences were not found for various elements, all in pref_control_behavior.js:38

Original issue's description:
> MD Settings: Adding some unit tests for <settings-main>.
>
> Specifically testing the "no results" message is shown/hidden as expected.
>  - Splitting SearchManager to an interface and an implementation.
>  - Using a TestSearchManager class (implements SearchManager) for testing
>    <settings-main>.
>
> BUG= 630383 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Committed: https://crrev.com/bb8fba2c2dbc0b02555f11877f4929786eab456c
> Cr-Commit-Position: refs/heads/master@{#409615}

TBR=michaelpg@chromium.org,dpapad@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 630383 

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

[modify] https://crrev.com/563a4d1a4a02714746f0782cafea9c5ad247c4ae/chrome/browser/resources/settings/search_settings.js
[modify] https://crrev.com/563a4d1a4a02714746f0782cafea9c5ad247c4ae/chrome/browser/resources/settings/settings_main/settings_main.html
[modify] https://crrev.com/563a4d1a4a02714746f0782cafea9c5ad247c4ae/chrome/browser/resources/settings/settings_main/settings_main.js
[modify] https://crrev.com/563a4d1a4a02714746f0782cafea9c5ad247c4ae/chrome/browser/resources/settings/settings_page/main_page_behavior.js
[modify] https://crrev.com/563a4d1a4a02714746f0782cafea9c5ad247c4ae/chrome/test/data/webui/settings/cr_settings_browsertest.js
[delete] https://crrev.com/3a4f654ac5c0ddd30f9194531add5b108f43d1dd/chrome/test/data/webui/settings/settings_main_test.js

Status: Assigned (was: Fixed)
Re-opening this, since the CL that added tests got reverted.
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 4 2016

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

commit 1c22a24eea98efce6f07c71706668cbc2ca71b3f
Author: dpapad <dpapad@chromium.org>
Date: Thu Aug 04 20:30:04 2016

Relanding: MD Settings: Adding some unit tests for <settings-main>.

Initial commit was reverted at https://codereview.chromium.org/2217493002
because the tests failed when run in debug bots. The
issue causing this is now fixed.

Specifically testing the "no results" message is shown/hidden as expected.
  - Splitting SearchManager to an interface and an implementation.
  - Using a TestSearchManager class (implements SearchManager) for testing
    <settings-main>.

BUG= 630383 
NOPRESUBMIT=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/1c22a24eea98efce6f07c71706668cbc2ca71b3f/chrome/browser/resources/settings/search_settings.js
[modify] https://crrev.com/1c22a24eea98efce6f07c71706668cbc2ca71b3f/chrome/browser/resources/settings/settings_main/settings_main.html
[modify] https://crrev.com/1c22a24eea98efce6f07c71706668cbc2ca71b3f/chrome/browser/resources/settings/settings_main/settings_main.js
[modify] https://crrev.com/1c22a24eea98efce6f07c71706668cbc2ca71b3f/chrome/browser/resources/settings/settings_page/main_page_behavior.js
[modify] https://crrev.com/1c22a24eea98efce6f07c71706668cbc2ca71b3f/chrome/test/data/webui/settings/cr_settings_browsertest.js
[add] https://crrev.com/1c22a24eea98efce6f07c71706668cbc2ca71b3f/chrome/test/data/webui/settings/settings_main_test.js

Status: Fixed (was: Assigned)

Sign in to add a comment