MD Settings: Subpage briefly reappears overtop content at end of collapse animation |
||||||||
Issue description[I thought we had a bug on this; can't find it, if so.] When collapsing a section, the subpage fades out while the card collapses. When the card finishes collapsing, the subpage's content suddenly shows again, on top of everything on the page, at its full original height. It is still fading out, so it has a low opacity and disappears completely in a few frames. This issue was introduced by fixing issue 647487 (see discussion there). The overflow is visible because: 1. "overflow: hidden" is removed on "settings-section #card" when the collapse finishes; otherwise we'd have more overflow issues (like 647487). 2. the neon-animations (fade-in/out-animation) have their default timing of 500ms, but the section collapse lasts 250ms The reason the overflow only appears to be visible for a couple frames, instead of 250ms, is mostly because of the animation curve: the subpage is virtually invisible by ~350ms. Fixing (2) should be easy: create a custom configuration for these animations. But we'd still have a race condition, so I'd propose fixing (1) as well. I *think* that we can apply overflow: hidden to every settings-subpage, all the time, without negative consequences -- but we should confirm that, there's probably some scenario I haven't found in testing.
,
May 1 2017
,
May 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/32a9e92ea1f387e91a9f7b87e19ce48f3990404c commit 32a9e92ea1f387e91a9f7b87e19ce48f3990404c Author: michaelpg <michaelpg@chromium.org> Date: Fri May 05 03:09:04 2017 MD Settings: Shorten page fade transitions Expanding or collapsing a section takes 350ms, but fading the new content in and the old content out takes 500ms. This is a contributing factor to the overlap seen when closing a section, as the old section is still visible when the collapse animation has completed. Shortening the fade duration to match the expand/collapse animation makes that overlap much less likely to occur. It also makes the animations appear snappier, IMO -- if the page is still fading after expanding, it looks slugging. This re-implements the fade animations, rather than programmatically changing the neon-animated-pages' transition durations for the existing fade-in-animation/fade-out-animation. The latter is not possible due to https://github.com/PolymerElements/neon-animation/issues/209. R=dbeam@chromium.org BUG= 716680 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2856223007 Cr-Commit-Position: refs/heads/master@{#469582} [modify] https://crrev.com/32a9e92ea1f387e91a9f7b87e19ce48f3990404c/chrome/browser/resources/settings/animation/compiled_resources2.gyp [add] https://crrev.com/32a9e92ea1f387e91a9f7b87e19ce48f3990404c/chrome/browser/resources/settings/animation/fade_animations.html [add] https://crrev.com/32a9e92ea1f387e91a9f7b87e19ce48f3990404c/chrome/browser/resources/settings/animation/fade_animations.js [modify] https://crrev.com/32a9e92ea1f387e91a9f7b87e19ce48f3990404c/chrome/browser/resources/settings/settings_page/settings_animated_pages.html [modify] https://crrev.com/32a9e92ea1f387e91a9f7b87e19ce48f3990404c/chrome/browser/resources/settings/settings_page/settings_animated_pages.js [modify] https://crrev.com/32a9e92ea1f387e91a9f7b87e19ce48f3990404c/chrome/browser/resources/settings/settings_resources.grd
,
May 5 2017
With the low-risk patch we just landed, the issue is unlikely to occur in normal operation: the animations now play for the same duration. I'll try patching this into M59 and if it works the same way, I'll request a merge. When an animation is canceled, it still happens: 1. Open a subpage 2. Before the subpage finishes, press Alt-Left or browser back The collapse happens instantly, but the fade animation continues playing. We could address that issue, or land https://codereview.chromium.org/2848793004/, but that patch is riskier and could complicate future work.
,
May 10 2017
I'd like to merge #3 this into 59 -- any contention around that? It should be low-risk: the only effect is to reduce the duration of subpage fade animations so the flash doesn't happen.
,
May 11 2017
I'm fine with it but could also survive without it. This "ghosting" is a mildly bad visual bug.
,
May 15 2017
Requesting merge to 59 for fixing noticeable visual glitch in MD Settings.
,
May 15 2017
This bug requires manual review: There is .grd file changes and we are only 21 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 15 2017
CL listed at #3 listed settings_resources.grd file change. Is this require string translation? Please note String freeze for M59 was on Mar 31.
,
May 16 2017
govind@/abdulsyed@: no, the .grd change was not about translations https://codereview.chromium.org/2856223007/diff/60001/chrome/browser/resources/settings/settings_resources.grd
,
May 16 2017
Please let us know once this has been tested and verified in canary or Dev. We're planning an RC cut today at 4PM for tomorrow's Beta release.
,
May 16 2017
I've verified the fix in Linux Canary and in tip of tree.
,
May 16 2017
Approving merge for M59. This does not require localization, as confirmed in #10.
,
May 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/056d542cd1bdab430bd4d39df75e47cbc941cd87 commit 056d542cd1bdab430bd4d39df75e47cbc941cd87 Author: Dan Beam <dbeam@chromium.org> Date: Tue May 16 23:00:11 2017 MD Settings: Shorten page fade transitions Expanding or collapsing a section takes 350ms, but fading the new content in and the old content out takes 500ms. This is a contributing factor to the overlap seen when closing a section, as the old section is still visible when the collapse animation has completed. Shortening the fade duration to match the expand/collapse animation makes that overlap much less likely to occur. It also makes the animations appear snappier, IMO -- if the page is still fading after expanding, it looks slugging. This re-implements the fade animations, rather than programmatically changing the neon-animated-pages' transition durations for the existing fade-in-animation/fade-out-animation. The latter is not possible due to https://github.com/PolymerElements/neon-animation/issues/209. R=dbeam@chromium.org BUG= 716680 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2856223007 Cr-Original-Commit-Position: refs/heads/master@{#469582} Review-Url: https://codereview.chromium.org/2889653005 . Cr-Commit-Position: refs/branch-heads/3071@{#590} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/056d542cd1bdab430bd4d39df75e47cbc941cd87/chrome/browser/resources/settings/animation/compiled_resources2.gyp [add] https://crrev.com/056d542cd1bdab430bd4d39df75e47cbc941cd87/chrome/browser/resources/settings/animation/fade_animations.html [add] https://crrev.com/056d542cd1bdab430bd4d39df75e47cbc941cd87/chrome/browser/resources/settings/animation/fade_animations.js [modify] https://crrev.com/056d542cd1bdab430bd4d39df75e47cbc941cd87/chrome/browser/resources/settings/settings_page/settings_animated_pages.html [modify] https://crrev.com/056d542cd1bdab430bd4d39df75e47cbc941cd87/chrome/browser/resources/settings/settings_page/settings_animated_pages.js [modify] https://crrev.com/056d542cd1bdab430bd4d39df75e47cbc941cd87/chrome/browser/resources/settings/settings_resources.grd |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by michae...@chromium.org
, Apr 29 2017