Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(38)

Issue 2901953002: Omnibox UI Experiments: Add Vertical Layout experiment (title-on-top). (Closed)

Created:
3 years, 7 months ago by tommycli
Modified:
3 years, 7 months ago
CC:
chromium-reviews, tfarina, asvitkine+watch_chromium.org, jdonnelly+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Omnibox UI Experiments: Add Vertical Layout experiment (title-on-top). This is a minimal Views-only patch that renders the title above the origin. It doesn't change any styling yet, nor does it touch Cocoa. Those will be in followup CLs. BUG=725599 Review-Url: https://codereview.chromium.org/2901953002 Cr-Commit-Position: refs/heads/master@{#475119} Committed: https://chromium.googlesource.com/chromium/src/+/26317da136aba1d8f3c46cd8347d396894416c7d

Patch Set 1 #

Patch Set 2 : swap text in some circumstances to match mobile #

Total comments: 10

Patch Set 3 : address pkasting comments #

Patch Set 4 : remove stray includes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -32 lines) Patch
M chrome/browser/about_flags.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/flag_descriptions.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/flag_descriptions.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/omnibox/omnibox_result_view.cc View 1 2 6 chunks +69 lines, -32 lines 0 comments Download
M components/omnibox/browser/omnibox_field_trial.h View 1 chunk +1 line, -0 lines 0 comments Download
M components/omnibox/browser/omnibox_field_trial.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M tools/metrics/histograms/enums.xml View 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (14 generated)
tommycli
jdonnelly: Sending this CL out to for informational purposes. Screenshots and discussion in bug. Looks ...
3 years, 7 months ago (2017-05-23 19:03:52 UTC) #2
tommycli
pkasting: PTAL. See bug for discussion and screenshots. Thanks!
3 years, 7 months ago (2017-05-25 18:18:06 UTC) #4
Peter Kasting
LGTM https://codereview.chromium.org/2901953002/diff/20001/chrome/browser/ui/views/omnibox/omnibox_result_view.cc File chrome/browser/ui/views/omnibox/omnibox_result_view.cc (right): https://codereview.chromium.org/2901953002/diff/20001/chrome/browser/ui/views/omnibox/omnibox_result_view.cc#newcode392 chrome/browser/ui/views/omnibox/omnibox_result_view.cc:392: contents_max_width); Nit: I'd probably rework this to avoid ...
3 years, 7 months ago (2017-05-25 20:24:39 UTC) #5
tommycli
pkasting: thanks! https://codereview.chromium.org/2901953002/diff/20001/chrome/browser/ui/views/omnibox/omnibox_result_view.cc File chrome/browser/ui/views/omnibox/omnibox_result_view.cc (right): https://codereview.chromium.org/2901953002/diff/20001/chrome/browser/ui/views/omnibox/omnibox_result_view.cc#newcode392 chrome/browser/ui/views/omnibox/omnibox_result_view.cc:392: contents_max_width); On 2017/05/25 20:24:39, Peter Kasting wrote: ...
3 years, 7 months ago (2017-05-25 21:59:10 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2901953002/60001
3 years, 7 months ago (2017-05-26 20:44:37 UTC) #17
commit-bot: I haz the power
3 years, 7 months ago (2017-05-26 20:50:54 UTC) #20
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/26317da136aba1d8f3c46cd8347d...

Powered by Google App Engine
This is Rietveld 408576698