Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2653753009:
Remove refine icon from multi-PW suggestions (Closed)
DescriptionRemove refine icon from multi-PW suggestions
Currently, when a user is given a Physical Web overflow suggestion in
the omnibox, a refine icon is also displayed. Tapping the refine icon
will fill the omnibox with several titles rather than a URL. This
change removes the refine icon in that condition.
BUG=663842, 686300
Review-Url: https://codereview.chromium.org/2653753009
Cr-Commit-Position: refs/heads/master@{#446463}
Committed: https://chromium.googlesource.com/chromium/src/+/c9c42e9c5a72a3ac28d698e848fba2888a32e00f
Patch Set 1 #
Total comments: 2
Messages
Total messages: 20 (6 generated)
cco3@chromium.org changed reviewers: + mattreynolds@chromium.org, tedchoc@chromium.org
lgtm
mmocny@chromium.org changed reviewers: + mmocny@chromium.org
https://codereview.chromium.org/2653753009/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/omnibox/SuggestionView.java (right): https://codereview.chromium.org/2653753009/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/omnibox/SuggestionView.java:333: && suggestionType != OmniboxSuggestionType.PHYSICAL_WEB_OVERFLOW); Nit: would it make sense to add this extra clause directly to the sameAsTyped assignment above?
https://codereview.chromium.org/2653753009/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/omnibox/SuggestionView.java (right): https://codereview.chromium.org/2653753009/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/omnibox/SuggestionView.java:333: && suggestionType != OmniboxSuggestionType.PHYSICAL_WEB_OVERFLOW); On 2017/01/26 20:50:58, mmocny wrote: > Nit: would it make sense to add this extra clause directly to the sameAsTyped > assignment above? not really...then sameAsTyped wouldn't mean the suggestion is not the same as the user typed.
lgtm
The CQ bit was checked by cco3@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1485464793232490, "parent_rev":
"fce540a08089df75d18cd4d719bec9ca03e88ac5", "commit_rev":
"c9c42e9c5a72a3ac28d698e848fba2888a32e00f"}
Message was sent while issue was closed.
Description was changed from ========== Remove refine icon from multi-PW suggestions Currently, when a user is given a Physical Web overflow suggestion in the omnibox, a refine icon is also displayed. Tapping the refine icon will fill the omnibox with several titles rather than a URL. This change removes the refine icon in that condition. BUG=663842 ========== to ========== Remove refine icon from multi-PW suggestions Currently, when a user is given a Physical Web overflow suggestion in the omnibox, a refine icon is also displayed. Tapping the refine icon will fill the omnibox with several titles rather than a URL. This change removes the refine icon in that condition. BUG=663842 Review-Url: https://codereview.chromium.org/2653753009 Cr-Commit-Position: refs/heads/master@{#446463} Committed: https://chromium.googlesource.com/chromium/src/+/c9c42e9c5a72a3ac28d698e848fb... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/c9c42e9c5a72a3ac28d698e848fb...
Message was sent while issue was closed.
Thanks, Ted! Would you be able to land this in m57? git drover --branch 2987 --cherry-pick c9c42e9c5a72a3ac28d698e848fba2888a32e00f
Message was sent while issue was closed.
On 2017/01/26 23:48:29, cco3 wrote: > Thanks, Ted! Would you be able to land this in m57? > > git drover --branch 2987 --cherry-pick c9c42e9c5a72a3ac28d698e848fba2888a32e00f Hi Ted, would you be able to do this, or is there some other route we should take?
Message was sent while issue was closed.
On 2017/01/27 22:11:51, cco3 wrote: > On 2017/01/26 23:48:29, cco3 wrote: > > Thanks, Ted! Would you be able to land this in m57? > > > > git drover --branch 2987 --cherry-pick > c9c42e9c5a72a3ac28d698e848fba2888a32e00f > > Hi Ted, would you be able to do this, or is there some other route we should > take? Sure, can you get merge approval on the bug first though?
Message was sent while issue was closed.
Description was changed from ========== Remove refine icon from multi-PW suggestions Currently, when a user is given a Physical Web overflow suggestion in the omnibox, a refine icon is also displayed. Tapping the refine icon will fill the omnibox with several titles rather than a URL. This change removes the refine icon in that condition. BUG=663842 Review-Url: https://codereview.chromium.org/2653753009 Cr-Commit-Position: refs/heads/master@{#446463} Committed: https://chromium.googlesource.com/chromium/src/+/c9c42e9c5a72a3ac28d698e848fb... ========== to ========== Remove refine icon from multi-PW suggestions Currently, when a user is given a Physical Web overflow suggestion in the omnibox, a refine icon is also displayed. Tapping the refine icon will fill the omnibox with several titles rather than a URL. This change removes the refine icon in that condition. BUG=663842,686300 Review-Url: https://codereview.chromium.org/2653753009 Cr-Commit-Position: refs/heads/master@{#446463} Committed: https://chromium.googlesource.com/chromium/src/+/c9c42e9c5a72a3ac28d698e848fb... ==========
Message was sent while issue was closed.
On 2017/01/27 22:59:40, Ted C wrote: > On 2017/01/27 22:11:51, cco3 wrote: > > On 2017/01/26 23:48:29, cco3 wrote: > > > Thanks, Ted! Would you be able to land this in m57? > > > > > > git drover --branch 2987 --cherry-pick > > c9c42e9c5a72a3ac28d698e848fba2888a32e00f > > > > Hi Ted, would you be able to do this, or is there some other route we should > > take? > > Sure, can you get merge approval on the bug first though? Done! https://bugs.chromium.org/p/chromium/issues/detail?id=686300
Message was sent while issue was closed.
Hi Ted, I got approval. Would you be able to land this now?
Message was sent while issue was closed.
On 2017/01/30 19:53:24, cco3 wrote: > Hi Ted, I got approval. Would you be able to land this now? Done: https://codereview.chromium.org/2659353003
Message was sent while issue was closed.
On 2017/01/30 21:07:31, Ted C wrote: > On 2017/01/30 19:53:24, cco3 wrote: > > Hi Ted, I got approval. Would you be able to land this now? > > Done: > https://codereview.chromium.org/2659353003 Thank you! |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
