[Tab loading animation] Update favicon display animation to spec |
|||||||
Issue descriptionCurrently the favicon cross fades in with the placeholder. We would like to proceed with animating the favicon in from the top per the proposal. Original proposal: https://docs.google.com/presentation/d/1r_63uPXs--gqO-FcA8fmzEuPavWrU3YJBkSObpo-D60/edit#slide=id.g2eec932b24_0_30 Spec: https://docs.google.com/document/d/1zLDEQquWVCLmA28Ir-RBavjVhAyfs-4JEman65aNuO4/edit?usp=sharing
,
Nov 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c86c85c5f86a8261f7bd380146a57c1c5758099 commit 6c86c85c5f86a8261f7bd380146a57c1c5758099 Author: Peter Boström <pbos@chromium.org> Date: Wed Nov 28 00:54:52 2018 Animate in favicon from the top Adds a drop-in motion to the current fade-in effect. This (fade in and motion) is currently linear but expected to be updated in a later change after consulting UX. Bug: chromium:903806 Change-Id: Iebc44c281ad18be2c02a1e84903cc6d0ff64b84f Reviewed-on: https://chromium-review.googlesource.com/c/1352689 Reviewed-by: Sidney San Martín <sdy@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#611475} [modify] https://crrev.com/6c86c85c5f86a8261f7bd380146a57c1c5758099/chrome/browser/ui/views/tabs/tab_icon.cc [modify] https://crrev.com/6c86c85c5f86a8261f7bd380146a57c1c5758099/chrome/browser/ui/views/tabs/tab_icon.h
,
Nov 29
Great job, refinement from Helene per spec: - Opacity of the placeholder to fade out to 0 over 200ms rather than just disappearing.
,
Nov 30
Hi, two things I noticed: 1.) The drop-in animation sometimes doesn't show up - instead the favicon appears immediately. Couldn't identify when it is the case :( 2.) The drop-in animation stutters on my MacBookAir and isn't smooth every time, which feels a little bit distractive then in the corner of my eyes :( Thanks for listening :)
,
Nov 30
@mehmet, could you get a screen recording for us for both these cases? It should animate every time, even on refresh. Thanks.
,
Nov 30
Hi edwardjung: Please find the screencast attached. In the first tab you'll see that the drop-in stutters (like there is a short break in the animation). In the second tab, you'll see that the favicon appears without the drop-in animation (sorry the cursor was shortly on it). If you need more information, please let me know. Thanks, Mehmet
,
Nov 30
Okay thanks. That is a little weird. I see you are visiting a site where we will have cached the icon but it should still animate in. Does it happen to all sites which you've been to before or just specific ones like Google.de? What model of MBA do you have? I'll do a test on my aging one at home.
,
Nov 30
I am using a MacBookAir 11" Mid 2012 (Non-Retina). Yes, I see this mostly, when I am visiting a site on the second tab where the same icon is still animating in on the first tab. I also could repro with macrumors.com or youtube.com. Just let me know, if you need more info.
,
Nov 30
Okay I just checked this on my 2010 MBA 11" and I'm not experiencing this issue, the animation is being shown and smooth. Could you file a separate bug. Maybe try in incognito and a clean profile just to rule out extension interference.
,
Nov 30
I'd assume the break in the animation is because there's no frame update, they should be happening at 30fps, but could probably be starved by the loading page, for instance.
,
Dec 1
re#4 - re#10: I opened issue 910847 and issue 910848 . Thanks.
,
Dec 3
Re #10, it seems like this is a actual bug, per the steps outlined in issue 910848 , I was able to reproduce on my MBP. It's possible the slow loading page is taking up all the resources causing this jank.
,
Dec 3
Jank due to resource contention w/ other things running on of the machine is unfortunately way out of scope here. :( In the screen recording above, 2. not showing an animation is probably / possibly issue 900036.
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/76e02a5b59bd965376946da324fcabd671c12ee2 commit 76e02a5b59bd965376946da324fcabd671c12ee2 Author: Peter Boström <pbos@chromium.org> Date: Tue Dec 04 19:46:57 2018 Add fade-in/fade-out to the favicon placeholder Bug: chromium:903806 , chromium:907494 Change-Id: Iad7e8f22630b842071e16f606a9fb2632415d967 Reviewed-on: https://chromium-review.googlesource.com/c/1359300 Reviewed-by: Sidney San Martín <sdy@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#613655} [modify] https://crrev.com/76e02a5b59bd965376946da324fcabd671c12ee2/chrome/browser/ui/views/tabs/tab_icon.cc [modify] https://crrev.com/76e02a5b59bd965376946da324fcabd671c12ee2/chrome/browser/ui/views/tabs/tab_icon.h
,
Dec 5
Requesting merge for #14, verified in Canary.
,
Dec 6
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b9b7993b62b6fc90ce6610f65ee59d53d5deedc2 commit b9b7993b62b6fc90ce6610f65ee59d53d5deedc2 Author: Peter Boström <pbos@chromium.org> Date: Thu Dec 06 17:54:45 2018 Add fade-in/fade-out to the favicon placeholder Bug: chromium:903806 , chromium:907494 Change-Id: Iad7e8f22630b842071e16f606a9fb2632415d967 Reviewed-on: https://chromium-review.googlesource.com/c/1359300 Reviewed-by: Sidney San Martín <sdy@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613655}(cherry picked from commit 76e02a5b59bd965376946da324fcabd671c12ee2) Reviewed-on: https://chromium-review.googlesource.com/c/1365978 Reviewed-by: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#116} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/b9b7993b62b6fc90ce6610f65ee59d53d5deedc2/chrome/browser/ui/views/tabs/tab_icon.cc [modify] https://crrev.com/b9b7993b62b6fc90ce6610f65ee59d53d5deedc2/chrome/browser/ui/views/tabs/tab_icon.h
,
Dec 6
I'll mark this as fixed as it's covering too much including M72 branches. Non-linear favicon drop in is now issue 912620 .
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b9b7993b62b6fc90ce6610f65ee59d53d5deedc2 Commit: b9b7993b62b6fc90ce6610f65ee59d53d5deedc2 Author: pbos@chromium.org Commiter: pbos@chromium.org Date: 2018-12-06 17:54:45 +0000 UTC Add fade-in/fade-out to the favicon placeholder Bug: chromium:903806 , chromium:907494 Change-Id: Iad7e8f22630b842071e16f606a9fb2632415d967 Reviewed-on: https://chromium-review.googlesource.com/c/1359300 Reviewed-by: Sidney San Martín <sdy@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613655}(cherry picked from commit 76e02a5b59bd965376946da324fcabd671c12ee2) Reviewed-on: https://chromium-review.googlesource.com/c/1365978 Reviewed-by: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#116} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by edwardjung@chromium.org
, Nov 23