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

Issue 901133003: Fix janky experimental app list animation. (Closed)

Created:
5 years, 10 months ago by calamity
Modified:
5 years, 10 months ago
Reviewers:
Matt Giuca
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@search_box_with_shadow
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix janky experimental app list animation. This CL fixes an issue where the experimental app list animation from the start page to the custom launcher page was janky because the bounds were being calculated incorrectly. This CL fixes the animation to use the size of the view being animated rather than the entire contents bounds. BUG=None Committed: https://crrev.com/610520d0568606787b0b995b44d104f35d8e7390 Cr-Commit-Position: refs/heads/master@{#314980}

Patch Set 1 #

Total comments: 2

Patch Set 2 : make animation have same velocity in both directions #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -4 lines) Patch
M ui/app_list/views/contents_animator.cc View 1 1 chunk +6 lines, -4 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
calamity
5 years, 10 months ago (2015-02-05 04:22:36 UTC) #2
Matt Giuca
https://codereview.chromium.org/901133003/diff/1/ui/app_list/views/contents_animator.cc File ui/app_list/views/contents_animator.cc (right): https://codereview.chromium.org/901133003/diff/1/ui/app_list/views/contents_animator.cc#newcode45 ui/app_list/views/contents_animator.cc:45: : contents_view_->GetContentsBounds().height()); As discussed, this should be + bounds.y().
5 years, 10 months ago (2015-02-05 05:51:54 UTC) #3
calamity
https://codereview.chromium.org/901133003/diff/1/ui/app_list/views/contents_animator.cc File ui/app_list/views/contents_animator.cc (right): https://codereview.chromium.org/901133003/diff/1/ui/app_list/views/contents_animator.cc#newcode45 ui/app_list/views/contents_animator.cc:45: : contents_view_->GetContentsBounds().height()); On 2015/02/05 05:51:54, Matt Giuca wrote: > ...
5 years, 10 months ago (2015-02-05 06:55:04 UTC) #4
Matt Giuca
lgtm
5 years, 10 months ago (2015-02-06 03:05:28 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/901133003/20001
5 years, 10 months ago (2015-02-06 04:34:38 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 10 months ago (2015-02-06 05:32:20 UTC) #8
commit-bot: I haz the power
5 years, 10 months ago (2015-02-06 05:33:26 UTC) #9
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/610520d0568606787b0b995b44d104f35d8e7390
Cr-Commit-Position: refs/heads/master@{#314980}

Powered by Google App Engine
This is Rietveld 408576698