Page MenuHomePhabricator

Unnecessary opacity animation for <h1> on load
Closed, ResolvedPublic8 Estimated Story Points

Description

To reproduce, record a timeline of VisualEditor initialization on http://en.wikipedia.beta.wmflabs.org/wiki/User:Etonkovidova. Notice that the opacity of
h1#firstHeading.firstHeading.ve-init-mw-viewPageTarget-transform.ve-init-mw-viewPageTarget-transform-muted.ve-init-mw-viewPageTarget-pageTitle is animated, costing (on my machine) 167.756ms.

The animation may be deliberate, but if we can eliminate it, we should.

Screen_Shot_2015-02-04_at_11.45.13.png (1×2 px, 561 KB)

Event Timeline

ori raised the priority of this task from to Needs Triage.
ori updated the task description. (Show Details)
ori added subscribers: ori, Catrope.
ori updated the task description. (Show Details)

It's a CSS animation though, not jQuery. And with no layout change impact afaik (only opacity).

The transition is specified in CSS as taking 200ms, but that shouldn't be a cost against something else as in this case I believe the transition is desired for UX and (other than general C/GPU cost) shouldn't be interfering with the javascript execution from the separate thread.

What's weird about this opacity transition is that opacity: 0.6; is applied two different ways:

  • To h1#firstHeading and div#siteSub through class="ve-init-mw-viewPageTarget-muted" and transitioned because of class="ve-init-mw-viewPageTarget-transform"
  • To div#jump-to-nav and div#mw-content-text through style="opacity: 0.6;" (i.e. inline style), with no transition

So by far the largest area that gets 60% opacity (the content area) doesn't get a transition, only a few little things do. That seems silly.

(Also we should probably not use a mix of inline styles and classes.)

I believe the transition is desired for UX and (other than general C/GPU cost) shouldn't be interfering with the javascript execution from the separate thread.

Depends on the browser. Not true for Firefox, even on the latest nightly.

See the experiment on http://www.phpied.com/css-animations-off-the-ui-thread/.

@Catrope Yeah, we should consolidate those. Maybe we can use an overall class and use descendant selectors instead of adding it to individual elements in JavaScript. Less DOM actions and ensures they start at the same time.

So instead of

.fade {
 opacity: 0.6;
}

$foo.addClass();
$bar.addClass();
$head.addClass();

$foo.removeClass();
$bar.removeClass();

.. we'd do

.ve-loading .foo,
.ve-loading .bar,
.ve-loading .head,
.ve-activated .head {
  opacity: 0.6;
}

$container.addClass();
$container.removeClass();
gerritbot added a subscriber: gerritbot.

Change 188733 had a related patch set uploaded (by Krinkle):
mw.ViewPageTarget: Use CSS instead of JS for DOM hiding/muting

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

Patch-For-Review

Change 188733 merged by jenkins-bot:
mw.ViewPageTarget: Use CSS instead of JS for DOM hiding/muting

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