Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2948403002:
Disabled overscroll animation when the fullscreen app list feature is enabled. (Closed)
DescriptionDisabled overscroll animation when the fullscreen app list feature is enabled.
BUG=735492
Review-Url: https://codereview.chromium.org/2948403002
Cr-Commit-Position: refs/heads/master@{#482296}
Committed: https://chromium.googlesource.com/chromium/src/+/ef8473e89d8066184b4811f3cfa50656e91a224f
Patch Set 1 : Disabled overscroll animation when the fullscreen app list feature is enabled. #
Total comments: 2
Patch Set 2 : Disabled overscroll animation when the fullscreen app list feature is enabled. #
Total comments: 4
Patch Set 3 : Disabled overscroll animation when the fullscreen app list feature is enabled. #
Messages
Total messages: 28 (22 generated)
Patchset #1 (id:1) has been deleted
Description was changed from ========== Disabled overscroll animation when the fullscreen app list feature is enabled. BUG=735492 ========== to ========== Disabled overscroll animation when the fullscreen app list feature is enabled. BUG=735492 ==========
newcomer@chromium.org changed reviewers: + vadimt@chromium.org, xiyuan@chromium.org
The CQ bit was checked by newcomer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
PTAL. -Alex
lgtm with one nit https://codereview.chromium.org/2948403002/diff/20001/ui/app_list/presenter/a... File ui/app_list/presenter/app_list_presenter_impl.cc (right): https://codereview.chromium.org/2948403002/diff/20001/ui/app_list/presenter/a... ui/app_list/presenter/app_list_presenter_impl.cc:280: if (is_fullscreen_app_list_enabled_ || nit: Move this to around line 272 and on its own "if" with the comment. The two are not really the same thing so prefer to have separate "if"s.
The CQ bit was checked by newcomer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2948403002/diff/40001/ui/app_list/presenter/a... File ui/app_list/presenter/app_list_presenter_impl.cc (right): https://codereview.chromium.org/2948403002/diff/40001/ui/app_list/presenter/a... ui/app_list/presenter/app_list_presenter_impl.cc:275: if(is_fullscreen_app_list_enabled_) Space after if. Run git cl format . Please create and maintain the list of repeated CR comments. https://codereview.chromium.org/2948403002/diff/40001/ui/app_list/presenter/a... File ui/app_list/presenter/app_list_presenter_impl.h (right): https://codereview.chromium.org/2948403002/diff/40001/ui/app_list/presenter/a... ui/app_list/presenter/app_list_presenter_impl.h:138: bool is_fullscreen_app_list_enabled_; const
The CQ bit was checked by newcomer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org Link to the patchset: https://codereview.chromium.org/2948403002/#ps40001 (title: "Disabled overscroll animation when the fullscreen app list feature is enabled.")
The CQ bit was unchecked by newcomer@chromium.org
The CQ bit was checked by newcomer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks all. -Alex https://codereview.chromium.org/2948403002/diff/20001/ui/app_list/presenter/a... File ui/app_list/presenter/app_list_presenter_impl.cc (right): https://codereview.chromium.org/2948403002/diff/20001/ui/app_list/presenter/a... ui/app_list/presenter/app_list_presenter_impl.cc:280: if (is_fullscreen_app_list_enabled_ || On 2017/06/23 20:02:44, xiyuan wrote: > nit: Move this to around line 272 and on its own "if" with the comment. The two > are not really the same thing so prefer to have separate "if"s. Done! https://codereview.chromium.org/2948403002/diff/40001/ui/app_list/presenter/a... File ui/app_list/presenter/app_list_presenter_impl.cc (right): https://codereview.chromium.org/2948403002/diff/40001/ui/app_list/presenter/a... ui/app_list/presenter/app_list_presenter_impl.cc:275: if(is_fullscreen_app_list_enabled_) On 2017/06/23 23:44:04, vadimt wrote: > Space after if. Run git cl format . > > Please create and maintain the list of repeated CR comments. Done. https://codereview.chromium.org/2948403002/diff/40001/ui/app_list/presenter/a... File ui/app_list/presenter/app_list_presenter_impl.h (right): https://codereview.chromium.org/2948403002/diff/40001/ui/app_list/presenter/a... ui/app_list/presenter/app_list_presenter_impl.h:138: bool is_fullscreen_app_list_enabled_; On 2017/06/23 23:44:04, vadimt wrote: > const Done.
The CQ bit was checked by newcomer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org, vadimt@chromium.org Link to the patchset: https://codereview.chromium.org/2948403002/#ps60001 (title: "Disabled overscroll animation when the fullscreen app list feature is enabled.")
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": 60001, "attempt_start_ts": 1498493642655290,
"parent_rev": "2efa616e207873fdbbf1ae8f144d16708b4a0d62", "commit_rev":
"ef8473e89d8066184b4811f3cfa50656e91a224f"}
Message was sent while issue was closed.
Description was changed from ========== Disabled overscroll animation when the fullscreen app list feature is enabled. BUG=735492 ========== to ========== Disabled overscroll animation when the fullscreen app list feature is enabled. BUG=735492 Review-Url: https://codereview.chromium.org/2948403002 Cr-Commit-Position: refs/heads/master@{#482296} Committed: https://chromium.googlesource.com/chromium/src/+/ef8473e89d8066184b4811f3cfa5... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/chromium/src/+/ef8473e89d8066184b4811f3cfa5... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
