[web-animations-1] Improvements to term definitions and cross-linking #8965
Conversation
There was a problem hiding this comment.
I really want to keep the definition of iteration progress at the point where we define how it is calculated. As a Web developer and implementer it's much more helpful to know what the value is and how it is calculated, rather than its significance in the model.
The rest looks great.
web-animations-1/Overview.bs
Outdated
| within a single iteration of an animation, | ||
| called the <em>iteration progress</em>. | ||
| The <em>iteration index</em> is also recorded | ||
| called the <dfn>iteration progress</dfn>. |
There was a problem hiding this comment.
The definition for iteration progress should remain in 4.11, not here.
There was a problem hiding this comment.
Ok, so reviewing the change... I disagree with putting the DFN in 4.11. :) I don't mind adding a cross-reference, but I put the DFN here for two reasons:
-
It's the actual defining instance of the term. 4.11 says how to calculate it (sorta), but doesn't say what it is.
-
For exactly that reason, 4.11 is confusing by itself. I land there and I don't understand what I'm looking at. I keep rewinding up the call stack, but I'm deep in the middle of a bunch of calculations for which I have no context. If it's a link back to the core definition, at least I can click on it to understand what's happening.
However, looking through the spec, moving the DFN to 4.9 might make sense (I haven't gotten that far yet); it would require some tweaking of the prose to introduce an actual definition, but it seems to have appropriate context for one. Wdyt?
There was a problem hiding this comment.
Ok, maybe I'm overreacting after encountering so many cases where CSS spec definitions just throw a term out there without actually defining it.
My understanding was this section was defining the timing model. The appearance of iteration progress was just to aid that description. The actual definition of the iteration should come later.
I'm not sure about 4.9 (the current draft has 4.9 as "4.9 Direction control" which I'm pretty sure it not what you meant). I could see "4.5 Animation effects" as a possibility, since the iteration progress is a property of an animation effect.
Alternatively, we could just beef up the description of the iteration progress in 4.11. For example, for the overall progress we have:
The overall progress describes the number of iterations that have completed (including partial iterations) and is defined as follows:
There was a problem hiding this comment.
Ok, maybe I'm overreacting after encountering so many cases where CSS spec definitions just throw a term out there without actually defining it.
That seems like an editorial error that we should fix. :)
I'm not sure about 4.9 (the current draft has 4.9 as "4.9 Direction control" which I'm pretty sure it not what you meant).
I was referencing https://www.w3.org/TR/web-animations-1/#core-animation-effect-calculations so actually, I guess it's 4.8 :)
So I think what I'll do is leave the DFN where it is for now, and then maybe switch it up once I get something to anchor it in maybe around 4.8. (since I'm going top down, essentially)
There was a problem hiding this comment.
Ok, fair enough. If you wanted to drop in a note so we don't forget to move the definition, that would be nice. I'd like to make sure we define the term properly in a suitable place, instead of it just being a side-effect of another definition.
|
Sorry for the dribs and drabs of comments here, I'm reviewing each commit separately (currently progress: 3 of 6). |
|
Ok, I've finished reviewing all the commits. They are really great. The details commit messages were very helpful. The only thing I feel strongly about is the definition of iteration progress. Everything else is fantastic. Thank you! |
…Animations API” Plus some nearby editorial fixes to the intro etc.
…imation model. Also better cross-linking.
…ents to timelines * Shift high-level description of timing model from “Time Values” section to “Timing Model” intro. * Dfn “timing node” since it's used without definition, and cross-link it from relevant places. * Rephrase “time value” dfn to work better with future time value types, stealing wording from the definition of “timeline”. * Clarify the purpose of a timeline. * Improve dfn of “unresolved” time values. * Add proper dfn for “active timeline”. * Clarify why document timeline offsets are called “origin time”. * Improve cross-linking throughout.
* Don't state the child relationship of animations to timelines as its informative definition; this is really confusing. It's not what an animation *is*, it's a side effect of the hierarchy. * Move the nice, readable, understandable definition of what an animation effect is to the definition of animation effect, where it's desperately needed! * Merge the remaining non-normative and normative definitions, which are half-duplicating each other already. * Pull up the definition of “current time”, because it's quite fundamental here. * Add a note about timeline vs animation notions of current time. * Split up the definition of animation into logical paragraphs: 1. binding effect and timeline (most fundamental) 2. Role in time value flow 3. Playback control features to manipulate said flow * Reorganize the remaining paragraphs into a DL. * Dfn-tag “seek” and cross-link it throughout
…he timeline definition No change to text (except headings). This section was making it hard to understand the “Timelines” section because it split the generic dfn from the specific types dfns with a much more complicated concept relating timelines to animations and events and other things. This moves it out of the way, and gives it a higher-level section heading (as it deserves).
The most fundamental function of an Animation is moderating the flow of time from the timeline to the Animation Effect. Understanding how this works is the most important thing, so pull it up.
Going through the spec and cleaning up things that tripped me up...
Might be easier to review by commit.
Some of the commit messages have detailed explanations, hope that helps...