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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 491973004: Make apps grid view scroll vertically in experimental app list. (Closed)

Created:
6 years, 4 months ago by Matt Giuca
Modified:
6 years, 3 months ago
Reviewers:
xiyuan, pkotwicz, calamity
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, tfarina, Jun Mukai
Base URL:
https://chromium.googlesource.com/chromium/src.git@calamity-ares-vertical-scroll
Project:
chromium
Visibility:
Public.

Description

Make apps grid view scroll vertically in experimental app list. Scrolling remains horizontal in the normal (and centered) app lists. BUG=406222 TEST=Run with --enable-experimental-app-list --show-app-list. Apps grid view should scroll vertically. Normal app list should still scroll horizontally. Committed: https://crrev.com/e322f3c3eae8adf9336efcf5c7923faac3528de5 Cr-Commit-Position: refs/heads/master@{#292826}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebase off CL 497413003. #

Patch Set 3 : Fixed gesture scrolling so it responds to vertical scrolling. #

Patch Set 4 : Touchpad scroll events: don't allow horizontal scrolling if view scrolling is vertical. #

Total comments: 1

Patch Set 5 : Various fixes. #

Total comments: 1

Patch Set 6 : AppListController: Fix overscroll axis. #

Total comments: 2

Patch Set 7 : Rebase. #

Patch Set 8 : Rename width to size. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -32 lines) Patch
M ash/wm/app_list_controller.cc View 1 2 3 4 5 1 chunk +6 lines, -1 line 0 comments Download
M ui/app_list/views/apps_grid_view.h View 1 2 2 chunks +5 lines, -0 lines 0 comments Download
M ui/app_list/views/apps_grid_view.cc View 1 2 3 4 5 6 7 8 chunks +76 lines, -31 lines 0 comments Download

Messages

Total messages: 22 (2 generated)
Matt Giuca
mgiuca@chromium.org changed reviewers: + calamity@chromium.org
6 years, 4 months ago (2014-08-26 05:40:48 UTC) #1
Matt Giuca
BAM!
6 years, 4 months ago (2014-08-26 05:40:48 UTC) #2
Matt Giuca
Patchset #3 (id:40001) has been deleted
6 years, 3 months ago (2014-08-28 03:02:02 UTC) #3
Matt Giuca
Patchset #3 (id:60001) has been deleted
6 years, 3 months ago (2014-08-28 03:03:49 UTC) #4
Matt Giuca
Fixed gesture scrolling. PTAL.
6 years, 3 months ago (2014-08-28 03:05:12 UTC) #5
Matt Giuca
Fixed touchpad scrolling. PTAL. https://codereview.chromium.org/491973004/diff/100001/ui/app_list/views/apps_grid_view.cc File ui/app_list/views/apps_grid_view.cc (right): https://codereview.chromium.org/491973004/diff/100001/ui/app_list/views/apps_grid_view.cc#newcode941 ui/app_list/views/apps_grid_view.cc:941: if (GetScrollAxis() == SCROLL_AXIS_HORIZONTAL) { ...
6 years, 3 months ago (2014-08-28 03:40:54 UTC) #6
Matt Giuca
https://codereview.chromium.org/491973004/diff/120001/ui/app_list/views/apps_grid_view.cc File ui/app_list/views/apps_grid_view.cc (right): https://codereview.chromium.org/491973004/diff/120001/ui/app_list/views/apps_grid_view.cc#newcode944 ui/app_list/views/apps_grid_view.cc:944: if (GetScrollAxis() == SCROLL_AXIS_HORIZONTAL) { Ugh, this duplication (with ...
6 years, 3 months ago (2014-08-28 08:35:21 UTC) #7
Matt Giuca
Note: Overscroll animation on ChromeOS is still horizontal. Need to fix that before committing this.
6 years, 3 months ago (2014-08-29 07:43:22 UTC) #8
Matt Giuca
mgiuca@chromium.org changed reviewers: + pkotwicz@chromium.org
6 years, 3 months ago (2014-08-29 08:04:25 UTC) #9
Matt Giuca
+pkotwicz: Please review ash AppListController.
6 years, 3 months ago (2014-08-29 08:04:25 UTC) #10
pkotwicz
Adding xiyuan@ as a reviewer who is more familiar with the app list than I ...
6 years, 3 months ago (2014-08-31 15:54:43 UTC) #12
pkotwicz
(if xiyuan@ is happy, I don't mind rubberstamping the CL)
6 years, 3 months ago (2014-08-31 15:55:54 UTC) #13
xiyuan
+mukai fyi lgtm https://codereview.chromium.org/491973004/diff/140001/ui/app_list/views/apps_grid_view.cc File ui/app_list/views/apps_grid_view.cc (right): https://codereview.chromium.org/491973004/diff/140001/ui/app_list/views/apps_grid_view.cc#newcode911 ui/app_list/views/apps_grid_view.cc:911: int width = GetScrollAxis() == SCROLL_AXIS_HORIZONTAL ...
6 years, 3 months ago (2014-08-31 16:08:32 UTC) #14
pkotwicz
Rubberstamp LGTM
6 years, 3 months ago (2014-08-31 20:39:17 UTC) #15
Matt Giuca
calamity: Did you still want to look at this or should I just put it ...
6 years, 3 months ago (2014-09-01 00:21:41 UTC) #16
calamity
lgtm https://codereview.chromium.org/491973004/diff/1/ui/app_list/views/apps_grid_view.cc File ui/app_list/views/apps_grid_view.cc (right): https://codereview.chromium.org/491973004/diff/1/ui/app_list/views/apps_grid_view.cc#newcode1139 ui/app_list/views/apps_grid_view.cc:1139: const int page_height = grid_rect.height() + kPagePadding; You ...
6 years, 3 months ago (2014-09-01 00:58:12 UTC) #17
Matt Giuca
https://codereview.chromium.org/491973004/diff/1/ui/app_list/views/apps_grid_view.cc File ui/app_list/views/apps_grid_view.cc (right): https://codereview.chromium.org/491973004/diff/1/ui/app_list/views/apps_grid_view.cc#newcode1139 ui/app_list/views/apps_grid_view.cc:1139: const int page_height = grid_rect.height() + kPagePadding; Nah, this ...
6 years, 3 months ago (2014-09-01 01:28:29 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mgiuca@chromium.org/491973004/180001
6 years, 3 months ago (2014-09-01 01:28:46 UTC) #20
commit-bot: I haz the power
Committed patchset #8 (id:180001) as 1a99b3f46671c8076f2f5ee9e8fdff9e1322452b
6 years, 3 months ago (2014-09-01 02:25:12 UTC) #21
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:15:22 UTC) #22
Message was sent while issue was closed. 
Patchset 8 (id:??) landed as
https://crrev.com/e322f3c3eae8adf9336efcf5c7923faac3528de5
Cr-Commit-Position: refs/heads/master@{#292826}

Powered by Google App Engine
This is Rietveld 408576698