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

Issue 2933673002: Preparing to making answer-card independent of fullscreen. (Closed)

Created:
3 years, 6 months ago by vadimt
Modified:
3 years, 5 months ago
Reviewers:
xiyuan, stevenjb
CC:
chromium-reviews, tfarina, Matt Giuca
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Preparing to making answer-card independent of fullscreen. We decided that it is possible to have answer cards in Launcher even before full-screen launcher is implemented. As answer cards dictate the design of the whole search result page, new search result page design will be applied when answer-card flag is on, even without the full-screen flag. The full-screen launcher comes with a total redesign of everything, including search results page, so the new design for search results page will be applied if full-screen flag is on, even without answer-card flag. This means that old design of search results page will be preserved only when both flags are off. Also, removing some unnecessary code. Bug=712331 Review-Url: https://codereview.chromium.org/2933673002 Cr-Commit-Position: refs/heads/master@{#478700} Committed: https://chromium.googlesource.com/chromium/src/+/2a35c6ab11910b5dd1cd63c5ac10b727f03492b3

Patch Set 1 #

Total comments: 2

Patch Set 2 : CR comments #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -8 lines) Patch
M ui/app_list/app_list_features.h View 1 chunk +1 line, -0 lines 2 comments Download
M ui/app_list/app_list_features.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M ui/app_list/views/app_list_main_view.cc View 1 1 chunk +1 line, -4 lines 0 comments Download
M ui/app_list/views/search_result_page_view.h View 1 chunk +2 lines, -2 lines 0 comments Download
M ui/app_list/views/search_result_page_view.cc View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 13 (6 generated)
vadimt
3 years, 6 months ago (2017-06-10 02:04:29 UTC) #2
xiyuan
lgtm https://codereview.chromium.org/2933673002/diff/1/ui/app_list/views/app_list_main_view.cc File ui/app_list/views/app_list_main_view.cc (right): https://codereview.chromium.org/2933673002/diff/1/ui/app_list/views/app_list_main_view.cc#newcode54 ui/app_list/views/app_list_main_view.cc:54: SetLayoutManager(static_cast<views::LayoutManager*>( Is static_cast still necessary?
3 years, 6 months ago (2017-06-12 16:16:13 UTC) #3
vadimt
https://codereview.chromium.org/2933673002/diff/1/ui/app_list/views/app_list_main_view.cc File ui/app_list/views/app_list_main_view.cc (right): https://codereview.chromium.org/2933673002/diff/1/ui/app_list/views/app_list_main_view.cc#newcode54 ui/app_list/views/app_list_main_view.cc:54: SetLayoutManager(static_cast<views::LayoutManager*>( On 2017/06/12 16:16:13, xiyuan wrote: > Is static_cast ...
3 years, 6 months ago (2017-06-12 18:13:11 UTC) #4
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/2933673002/20001
3 years, 6 months ago (2017-06-12 18:13:53 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/2a35c6ab11910b5dd1cd63c5ac10b727f03492b3
3 years, 6 months ago (2017-06-12 18:51:37 UTC) #10
stevenjb
https://codereview.chromium.org/2933673002/diff/20001/ui/app_list/app_list_features.h File ui/app_list/app_list_features.h (right): https://codereview.chromium.org/2933673002/diff/20001/ui/app_list/app_list_features.h#newcode34 ui/app_list/app_list_features.h:34: bool APP_LIST_EXPORT IsSearchResultsNewDesignEnabled(); This is an unfortunate name for ...
3 years, 5 months ago (2017-07-11 22:34:24 UTC) #12
vadimt
3 years, 5 months ago (2017-07-11 23:04:41 UTC) #13
Message was sent while issue was closed.
https://codereview.chromium.org/2933673002/diff/20001/ui/app_list/app_list_fe...
File ui/app_list/app_list_features.h (right):

https://codereview.chromium.org/2933673002/diff/20001/ui/app_list/app_list_fe...
ui/app_list/app_list_features.h:34: bool APP_LIST_EXPORT
IsSearchResultsNewDesignEnabled();
I'll rename it into "TouchFriendlySearchResults" in my next CL. Thanks!

Powered by Google App Engine
This is Rietveld 408576698