Change 561704 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/MinervaNeue@master] Remove use of animations CSS class from Minerva

https://gerrit.wikimedia.org/r/561704

Change 561705 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Remove Browser.supportsAnimations method/.animations CSS class

https://gerrit.wikimedia.org/r/561705

So I have two patches ready that should knock out the .animations class and the supportsAnimations method, but the only problem is I think this makes the chrome animation flash bug (https://bugs.chromium.org/p/chromium/issues/detail?id=332189) rear its ugly head again.

I've noticed the AMC overflow menu/user menu and main menu flash on page load sometimes with the patches. Solutions like https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/528265/ depend on JS setting the .animations class to avoid this flash. Therefore, I'm not really sure how to proceed or even if we want to proceed with removing this animations class from minerva/mobile frontend given this bug.

cc @Jdlrobson

I think this makes the chrome animation flash bug (https://bugs.chromium.org/p/chromium/issues/detail?id=332189) rear its ugly head again.

This seems to impact Chrome/31 which is not a grade A browser so I think this is okay?

@Jdlrobson I've been able to replicate it on the latest Chrome version 79.0.3945.88 (Mac) so I don't think the bug has been resolved yet. The bug ticket is still open too.

Can you post a video of what this animation flash looks like? Are there any ways we can avoid the flash by changing how we apply the animation? e.g. using focus state?

The video attached is what I'm seeing.

With the overflow menu, I think we had a previous workaround for this problem (before https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/528265/) where we sacrificed the exit animation in favor of not animating the visibility property (which is what makes the flash in chrome) but getting rid of the main menu exit animation would probably be more noticeable/jarring here.

I'm not really sure of a css only solution to avoid this problem and also keep the enter and exit animations rn, maybe @Jdrewniak knows?

I made a minimal test case of the bug at https://quizzical-bhabha-d9a97e.netlify.com/. Notice the red box moving down when the page loads in chrome - that shouldn't happen as the only css is the following:

div {
  width: 500px;
  height: 500px;
  background: red;
  transform: translateY(300px);
  transition: transform 1000ms ease-in-out;
}

Change 562393 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Add js added CSS class to body to prevent CSS transitions from firing on page load

https://gerrit.wikimedia.org/r/562393

@nray @Jdlrobson Yeah, I don't see how we can avoid this bug without adding a class on page load. Removing the animation check & tests is fine, but we'll still have to add a class to the body.

... actually, It might be a bit late to suggest this, but it just occurred to me that animation class on the body might be more useful than just a browser check.

It can instead be an accessibility check for the prefers-reduced-motion media query. This is an option users can set on iOS 10 & Android 9 (also Mac & windows) to reduce animations that may cause sickness for some users.

We can use a check like this before adding the animation class.

window.matchMedia('(prefers-reduced-motion)').matches

Change 562642 had a related patch set uploaded (by Nray; owner: Nick Ray):
[mediawiki/extensions/Echo@master] WIP: Make echo respect animations class

https://gerrit.wikimedia.org/r/562642

Change 562643 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/MinervaNeue@master] WIP: Split animations for NotificationsOverlay, transparent shield from rest of CSS

https://gerrit.wikimedia.org/r/562643

Change 562644 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/MinervaNeue@master] WIP: Make animations class respect user system settings

https://gerrit.wikimedia.org/r/562644

Change 562644 abandoned by Nray:
WIP: Make animations class respect user system settings

Reason:
This should probably go in core conidering we have animations across MobileFrontend (search overlay, watchstar) and Minerva (MainMenu, ToggleList)

https://gerrit.wikimedia.org/r/562644

Change 562643 abandoned by Nray:
WIP: Split animations for NotificationsOverlay, transparent shield from rest of CSS

Reason:
This should probably go in core conidering we have animations across MobileFrontend (search overlay, watchstar) and Minerva (MainMenu, ToggleList)

https://gerrit.wikimedia.org/r/562643

Change 562642 abandoned by Nray:
WIP: Make echo respect animations class

https://gerrit.wikimedia.org/r/562642

@Jdrewniak That's a great idea, and I took a shot at doing this in:

https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/562643/
https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/562644/
https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Echo/+/562642/

But abandoned it after realizing that we have animations across MobileFrontend (search overlay, watchstar) and Minerva (MainMenu, ToggleList) and some extensions like Echo rely on the transitionend event firing to complete their logic so a class like that would probably be best in core but that would add significant scope creep to this ticket (and I'm not sure what the priority of that would be in relation to everything else). So I think I'll instead add an animations class in minerva that will avoid the chrome bug and call this ticket done

🤷‍♂️ oh well...

Change 562393 abandoned by Nray:
Add js added CSS class to body to prevent CSS transitions from firing on page load

https://gerrit.wikimedia.org/r/562393

Patches to review:

https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/561704/
https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/561705/
https://gerrit.wikimedia.org/r/563002
https://gerrit.wikimedia.org/r/563260

Change 563002 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/MinervaNeue@master] Remove 'animated' class from toast

https://gerrit.wikimedia.org/r/563002

Change 563260 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Remove animated class usage from Drawer

https://gerrit.wikimedia.org/r/563260

Change 561704 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Add minerva-animations-ready CSS class in Minerva

https://gerrit.wikimedia.org/r/561704

Change 563002 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove 'animated', mw-notification-tag-toast classes from toast.less

https://gerrit.wikimedia.org/r/563002

Change 561705 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove Browser.supportsAnimations method/.animations CSS class

https://gerrit.wikimedia.org/r/561705

Change 563260 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove animated class usage from Drawer

https://gerrit.wikimedia.org/r/563260

Test Result

Status: ✅ PASS
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):

QA steps

✅ AC1: When logged out click watchstar. Ensure the opening and closing of the drawer is animated
✅ AC2: Open the menu and close it - confirming it is animated.

✅ AC3: Log in and click the notification button in the top right on a desktop device. Confirm it is animated.

Change 599431 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/Vector@master] [fix][Less] disable sidebar animations on page load

https://gerrit.wikimedia.org/r/599431

In T234570#5781554, @Jdrewniak wrote:

@nray @Jdlrobson Yeah, I don't see how we can avoid this bug without adding a class on page load. Removing the animation check & tests is fine, but we'll still have to add a class to the body.

... actually, It might be a bit late to suggest this, but it just occurred to me that animation class on the body might be more useful than just a browser check.

It can instead be an accessibility check for the prefers-reduced-motion media query. This is an option users can set on iOS 10 & Android 9 (also Mac & windows) to reduce animations that may cause sickness for some users.

We can use a check like this before adding the animation class.

window.matchMedia('(prefers-reduced-motion)').matches

👆🏼It would be formidable to add those checks to Vector!

Core please given the recent activity in Vector :) Not vector! Seems useful to have these for all skins.

Change 599431 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [fix][Less] disable sidebar animations on page load

https://gerrit.wikimedia.org/r/599431