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

Issue 2965813002: Added flag in chrome://flags to enable the New Launcher UI. (Closed)

Created:
3 years, 5 months ago by newcomer
Modified:
3 years, 5 months ago
Reviewers:
stevenjb
CC:
chromium-reviews, tfarina, Matt Giuca, asvitkine+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Added flag in chrome://flags to enable the New Launcher UI. BUG=738573 Review-Url: https://codereview.chromium.org/2965813002 Cr-Commit-Position: refs/heads/master@{#484314} Committed: https://chromium.googlesource.com/chromium/src/+/403ac1400f04a1a924811cf8515046ad58e73013

Patch Set 1 : Added flag in chrome://flags to enable the feature. #

Total comments: 6

Patch Set 2 : Added flag in chrome://flags to enable the feature. #

Total comments: 2

Patch Set 3 : Edited flag description. #

Patch Set 4 : Added flag to histograms/enums.xml #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -1 line) Patch
M chrome/browser/about_flags.cc View 1 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/flag_descriptions.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/flag_descriptions.cc View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M tools/metrics/histograms/enums.xml View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M ui/app_list/app_list_features.cc View 1 2 chunks +3 lines, -1 line 0 comments Download
M ui/app_list/app_list_switches.h View 1 2 chunks +3 lines, -0 lines 0 comments Download
M ui/app_list/app_list_switches.cc View 1 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 27 (20 generated)
newcomer
PTAL! -Alex
3 years, 5 months ago (2017-06-30 21:04:22 UTC) #4
stevenjb
https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/about_flags.cc#newcode1445 chrome/browser/about_flags.cc:1445: SINGLE_VALUE_TYPE(app_list::switches::kEnableNewAppListUI), Is there a better descriptor here than "new"? ...
3 years, 5 months ago (2017-06-30 21:17:26 UTC) #7
newcomer
Thanks for the feedback! Changed variable names. -Alex https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/2965813002/diff/20001/chrome/browser/about_flags.cc#newcode1445 chrome/browser/about_flags.cc:1445: SINGLE_VALUE_TYPE(app_list::switches::kEnableNewAppListUI), ...
3 years, 5 months ago (2017-06-30 21:40:24 UTC) #11
stevenjb
lgtm w/ one nit/suggestion https://codereview.chromium.org/2965813002/diff/100001/chrome/browser/flag_descriptions.cc File chrome/browser/flag_descriptions.cc (right): https://codereview.chromium.org/2965813002/diff/100001/chrome/browser/flag_descriptions.cc#newcode2934 chrome/browser/flag_descriptions.cc:2934: "features."; 'launcher' is fine in ...
3 years, 5 months ago (2017-06-30 22:18:59 UTC) #12
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/2965813002/140001
3 years, 5 months ago (2017-07-05 18:19:52 UTC) #23
commit-bot: I haz the power
Committed patchset #4 (id:140001) as https://chromium.googlesource.com/chromium/src/+/403ac1400f04a1a924811cf8515046ad58e73013
3 years, 5 months ago (2017-07-05 18:26:49 UTC) #26
newcomer
3 years, 5 months ago (2017-07-11 19:54:05 UTC) #27
Message was sent while issue was closed.
https://codereview.chromium.org/2965813002/diff/100001/chrome/browser/flag_de...
File chrome/browser/flag_descriptions.cc (right):

https://codereview.chromium.org/2965813002/diff/100001/chrome/browser/flag_de...
chrome/browser/flag_descriptions.cc:2934: "features.";
On 2017/06/30 22:18:58, stevenjb wrote:
> 'launcher' is fine in these strings, although I have no idea what 'Peeking'
> means; presumably folks using this switch will know?
> 
> s/The new launcher/The peeking launcher/ ?

Good catch!

Powered by Google App Engine
This is Rietveld 408576698