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

Issue 633657 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 608535



Sign in to add a comment

MD Settings: Highlight sub-level search hits.

Project Member Reported by dpa...@chromium.org, Aug 2 2016

Issue description

Started working on this issue yesterday. Experimental CL at https://codereview.chromium.org/2202723004.

There are two sub-problems to be solved.

Subproblem 1: Associate subpages with the HTML control that needs to be
clicked/tapped to open the subpage.

This information can't be inferred by simply inspecting the DOM structure,
because the subpage and the control that triggers it are not in a parent-child
relationship. Instead they can be siblings (for example see [1] for the search
engines subpage).
  It is worth noting that the old Options had faced the same problem, and it was
solved by programmatically designating overlays and associated controls, see [2].
Then when a search hit is found, the associated control can be located and
highlighted. The experimental CL above is following similar approach.

Subproblem 2: Implement sub-level highlight UI.
Mocks exist at [3]. There is a problem though. The mocks show the highlight
bubble on the left and outside of the settings box (white rectangle). There is
no space to place it there anymore (see attachment with alternative approach).

@tbuckley, @bettes: Could you please advise on how to proceed with regards to
the search bubble placement problem?


[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/search_page/search_page.html?q=search_page.html&sq=package:chromium&dr&l=43,35
[2] https://cs.chromium.org/chromium/src/ui/webui/resources/js/cr/ui/page_manager/page_manager.js?q=associatedControls&sq=package:chromium&l=98&dr=C
[3] https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20Inner%20Pages/03-Settings/specs#%2FSPEC-settings_searchUI.png%3Fz=width

 
highlight_right.png
16.3 KB View Download
Labels: -Pri-2 Pri-1
Owner: bettes@chromium.org
Status: Assigned (was: Untriaged)
Could we simply remove the text from the tooltip but leave it in the margin? Can we fit something like "..." in the bubble in that case? The user knows what they typed, so showing it all over the page doesn't seem necessary.
@tbuckley: IIUC, your suggestion looks like this, http://imgur.com/a/CFgBY. In the case where there is not enough space, the three dots are not visible, but the right side of the search bubble is (is that enough though?).
Status update: I have implemented locally what the current mocks ask for (in 2 CLs)

1) https://codereview.chromium.org/2210583002 (associates a subpage with the control that triggers it).
2) https://codereview.chromium.org/2202723004 (Modifies search algorithm to show/hide search bubbles).

The search bubble is dynamically positioned such that it always aligns with the edge of the white box, see http://imgur.com/a/BAQdj. I have not addressed the case where there is not enough space to show the bubble, still
waiting on @bettes to come up with a suggestion for those cases. Also have not addressed RTL yet (bubble currently
shows up on the left in both LTR and RTL).



Project Member

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

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

commit ca4931526f851d5eb74bcb22b85af36a260f12c2
Author: dpapad <dpapad@chromium.org>
Date: Fri Aug 05 03:06:34 2016

MD Settings: Associate suppages with the controls that trigger them.

This is necessary such that when a search hit is found in a subpage the
associated control can be properly highlighted to guide the user to the search
hit.

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

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

[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/appearance_page/appearance_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/device_page/device_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/internet_page/internet_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/languages_page/languages_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/passwords_and_forms_page/passwords_and_forms_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/people_page/people_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/printing_page/printing_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/privacy_page/privacy_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/search_page/search_page.html
[modify] https://crrev.com/ca4931526f851d5eb74bcb22b85af36a260f12c2/chrome/browser/resources/settings/settings_page/settings_subpage.js

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 5 2016

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

commit 52c0e1b022a26550030b711b7e2019a4a0bfa3d4
Author: dpapad <dpapad@chromium.org>
Date: Fri Aug 05 19:18:15 2016

MD Settings: Highlight sub-page search hits.

Specifically:
 - When a search hit occurs in a subpage, display a search bubble
   next to the element that is desginated as the "associated control"
   The search bubble guides the user to find the search hit.
 - Address case where multiple search hits occur in the same supgage,
   by ensuring that only one search bubble is added.
 - Update "unhighlight" codepath to also remove search bubbles.

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

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

[modify] https://crrev.com/52c0e1b022a26550030b711b7e2019a4a0bfa3d4/chrome/browser/resources/settings/search_settings.js
[modify] https://crrev.com/52c0e1b022a26550030b711b7e2019a4a0bfa3d4/chrome/browser/resources/settings/settings_page/settings_animated_pages.html
[modify] https://crrev.com/52c0e1b022a26550030b711b7e2019a4a0bfa3d4/chrome/browser/resources/settings/settings_shared_css.html

Labels: Hotlist-MD-Settings-SearchBox

Comment 7 by dpa...@chromium.org, Aug 10 2016

Cc: nyerramilli@chromium.org dpa...@chromium.org ranjitkan@chromium.org
 Issue 636246  has been merged into this issue.

Comment 8 by bettes@chromium.org, Aug 11 2016

I think the best thing to do here is to revert back to how tooltips operate today, but with the new visuals:

- displayed above the item and consequently cover up certain elements of the page. 
- keep the same "avoid mouse" behavior that we have today



Comment 9 by dpa...@chromium.org, Aug 12 2016

Cc: tommycli@chromium.org
Owner: dpa...@chromium.org
Owner: tommycli@chromium.org
Since dpapad@ is OOO, I'll take this bug on.
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 22 2016

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

commit 3bae501dc4cb4c7df5739f15efb7c5edb22b90ca
Author: tommycli <tommycli@chromium.org>
Date: Mon Aug 22 20:27:15 2016

Settings Search: Make tooltip vertical.

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

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

[modify] https://crrev.com/3bae501dc4cb4c7df5739f15efb7c5edb22b90ca/chrome/browser/resources/settings/search_settings.js
[modify] https://crrev.com/3bae501dc4cb4c7df5739f15efb7c5edb22b90ca/chrome/browser/resources/settings/settings_shared_css.html

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 22 2016

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

commit 1886c57203a481f6818529a50c82dcc753dacdef
Author: tommycli <tommycli@chromium.org>
Date: Mon Aug 22 23:34:06 2016

Settings Search: Make tooltip run away from mouse.

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

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

[modify] https://crrev.com/1886c57203a481f6818529a50c82dcc753dacdef/chrome/browser/resources/settings/search_settings.js
[modify] https://crrev.com/1886c57203a481f6818529a50c82dcc753dacdef/chrome/browser/resources/settings/settings_shared_css.html

Status: Fixed (was: Assigned)
Dis done.

Sign in to add a comment