Closed Bug 1553021 Opened 5 years ago Closed 5 years ago

Update getAnimations parameter to match latest spec

Categories

(Core :: DOM: Animation, task, P3)

Product:
Core
Component:
DOM: Animation
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla69
Tracking Flags:
Tracking Status
firefox69 --- fixed

People

(Reporter: birtles, Assigned: birtles)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

Bug 1553021 - Update naming of parameter to getAnimations to match spec; r=bzbarsky
47 bytes, text/x-phabricator-request
Details | Review
Bug 1553021 - Add web-platform-tests for { subtree: true } option to Animatable.getAnimations; r=emilio
47 bytes, text/x-phabricator-request
Details | Review

We have implemented a { subtree: true } flag in Animatable.getAnimations() for quite a while but I've only recently specified it. (We don't ship getAnimations() anywhere yet.)

The spec text uses slightly different naming for this parameter so this bug is just about bringing our naming up-to-date and adding a few WPT for it.

Chris, I'd like to document this method on MDN but I'm a bit unsure how to proceed.

The situation is:

  • There is a getAnimations() method on Document which is documented here: Document.getAnimations()
  • There is also a getAnimations() method on the Animatable mixin interface. This is not documented anywhere.
  • This bug adds a parameter to getAnimations() on Animatable (not Document).
  • The Animatable mixin interface also defines the animate() method.
  • Element and CSSPSeudoElement implement the Animatable mixin interface.
  • Currently we document animate() as it applies to Element on this page: Element.animate().
  • Technically I guess animate() should be on the Animatable mixin interface but probably 99.9% of the time people use it, they use it on an Element and I'm not sure if MDN supports mixins. I would want to make sure that when a user looks up Element's methods, animate() shows up there.

So should I just add a page Element.getAnimations() ? Or should we move Element.animate() to Animatable/animate and link to it from Element somehow?

For what's its worth, I started writing the introduction to such a page:

The getAnimations() method of the Animatable interface returns an array of all Animation objects affecting this element or which are scheduled to do so in future. It can optionally return Animation objects for descendant elements too. This array includes CSS Animations, CSS Transitions, and Web Animations.

Flags: needinfo?(cmills)
Keywords: dev-doc-needed

So should I just add a page Element.getAnimations() ? Or should we move Element.animate() to Animatable/animate and link to it from Element somehow?

Hi Brian! I would just add the Element.getAnimations() page. We don't have a proper mechanism to support mixins currently, and have in the past opted to document them just like regular interfaces and just duplicate the lists of methods/properties on the pages for the interfaces that include them. But there is evidence that MDN readers are confused by this. Mixins are a spec device that really shouldn't be thust upon regular web devs.

Really what we need is some kind of inheritance mechanism that allows us to define the mixin content somewhere, say what implements it, and then have it show up just on the pages for the implementing interfaces. But we're not there yet.

If you are concerned about making sure the inheritance is specified somewhere, you could always put a note in the "Specifications" section that says something about getAnimations() being specified on the Animatable mixin, which is implemented by Element and CSSPseudoElement (which we've not documented yet...).

Flags: needinfo?(cmills)

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #4)

So should I just add a page Element.getAnimations() ? Or should we move Element.animate() to Animatable/animate and link to it from Element somehow?

Hi Brian! I would just add the Element.getAnimations() page. We don't have a proper mechanism to support mixins currently, and have in the past opted to document them just like regular interfaces and just duplicate the lists of methods/properties on the pages for the interfaces that include them. But there is evidence that MDN readers are confused by this. Mixins are a spec device that really shouldn't be thust upon regular web devs.

Makes sense. Thanks!

I've added https://developer.mozilla.org/en-US/docs/Web/API/Element/getAnimations for now and tidied up a few other pages so they link to it as appropriate. I think I still need to add the BCD entry however.

Really what we need is some kind of inheritance mechanism that allows us to define the mixin content somewhere, say what implements it, and then have it show up just on the pages for the implementing interfaces. But we're not there yet.

Yep, that sounds good, but it also sounds like quite a bit of work to make happen.

If you are concerned about making sure the inheritance is specified somewhere, you could always put a note in the "Specifications" section that says something about getAnimations() being specified on the Animatable mixin, which is implemented by Element and CSSPseudoElement (which we've not documented yet...).

I've linked do the definition on the Animatable mixin so if authors are curious about where and how it is defined they can find it from there.

Pushed by bbirtles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8b8349028050
Update naming of parameter to getAnimations to match spec; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/68b958d1b1e9
Add web-platform-tests for { subtree: true } option to Animatable.getAnimations; r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16967 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

I've added https://developer.mozilla.org/en-US/docs/Web/API/Element/getAnimations for now and tidied up a few other pages so they link to it as appropriate. I think I still need to add the BCD entry however.

I've had a look, and given it a small copy edit. Looks great — thanks for doing that Brian.

https://hg.mozilla.org/mozilla-central/rev/8b8349028050
https://hg.mozilla.org/mozilla-central/rev/68b958d1b1e9

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox69: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: