New issue
Advanced search Search tips

Issue 803126 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Native UI highlights matched search queries

Project Member Reported by wutao@chromium.org, Jan 17 2018

Issue description

Currently I am thinking to use  base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents to return the first matched query offset.

About the UI, there are two options:
1. Use StyledLabel to set the range, when I prototyping it, I find I need to clear the style_ranges_ for each query. So we need to exposure a function for style_ranges_.clear.

https://cs.chromium.org/chromium/src/ui/views/controls/styled_label.h?l=171&rcl=8927a8f42aca203f80c0bd258e637936e65d0c41

2. We can also use render text RenderText::ApplyColor(SkColor value, const Range& range) and ApplyStyle to set the matched styles.
https://cs.chromium.org/chromium/src/ui/gfx/render_text.cc?l=694&rcl=0345c402fff1f9e3b389b6b1891aa0f90bc1598f


Option 1 looks easier, but it cannot be used together with other ranges styles in the text because it will clear the style_ranges_ for each query. Unless we have two style_ranges_ and search_highlight_reanges_?
 

Comment 1 by msw@chromium.org, Jan 17 2018

Do you plan to use the search highlighting together with other ranged styles?
(maybe clearing the styles and just re-applying search highlighting isn't a problem)

Otherwise, some random ideas: (1) clear just one type of style (ie. bolding), and leave other styles alone, (2) clear all styles and re-apply the non-search styling with a KSV helper function, (3) use separate view copies to show styled text with search highlights and keep the originals...

Comment 2 by wutao@chromium.org, Jan 17 2018

Thanks msw@. in current UX mock of KSV, we do not need to use together with other ranged styles. We only highlight the left column text in the mock, not the right column text with the bubbles.

Even in the future we need to use together, your random ideas would make it work!
Sounds like a very small cl for this feature. Thanks.
Status: Fixed (was: Available)

Sign in to add a comment