Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates related to the animation-composition property #19940

Merged
merged 5 commits into from Sep 1, 2022

Conversation

dipikabh
Copy link
Contributor

@dipikabh dipikabh commented Aug 24, 2022

Summary

Related issues

Doc issue tracker: #18771

Metadata

  • Adds a new document
  • Rewrites (or significantly expands) a document
  • Fixes a typo, bug, or other error

@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs Content:WebAPI Web API docs labels Aug 24, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 24, 2022

Preview URLs

Flaws

Note! 8 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/CSS/CSS_Animations
Title: CSS Animations
on GitHub
Flaw count: 1

  • macros:
    • /en-US/docs/Web/CSS/animation-composition does not exist

URL: /en-US/docs/Web/CSS/CSS_Animations/Using_CSS_animations
Title: Using CSS animations
on GitHub
Flaw count: 2

  • macros:
    • /en-US/docs/Web/CSS/animation-composition does not exist
    • /en-US/docs/Glossary/Composite_operation does not exist

URL: /en-US/docs/Web/API/AnimationEvent
Title: AnimationEvent
on GitHub
Flaw count: 1

  • macros:
    • /en-US/docs/Web/CSS/animation-composition does not exist

External URLs

URL: /en-US/docs/Web/CSS/animation-fill-mode
Title: animation-fill-mode
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/animation-name
Title: animation-name
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/animation-timing-function
Title: animation-timing-function
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/animation-delay
Title: animation-delay
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/animation-play-state
Title: animation-play-state
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/CSS_Animations
Title: CSS Animations
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/CSS_Animations/Using_CSS_animations
Title: Using CSS animations
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/animation-iteration-count
Title: animation-iteration-count
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/animation-direction
Title: animation-direction
on GitHub

No new external URLs


URL: /en-US/docs/Web/CSS/animation-duration
Title: animation-duration
on GitHub

No new external URLs


URL: /en-US/docs/Web/API/AnimationEvent
Title: AnimationEvent
on GitHub

No new external URLs

(this comment was updated 2022-09-01 11:00:39.150240)

@dipikabh dipikabh marked this pull request as ready for review August 24, 2022 22:11
@dipikabh dipikabh requested review from a team as code owners August 24, 2022 22:11
@dipikabh dipikabh requested review from Elchi3 and estelle and removed request for a team August 24, 2022 22:11

```css
animation-name: fadeInOut, moveLeft300px, bounce;
animation-duration: 2.5s, 5s, 1s;
animation-iteration-count: 2, 1, 5;
```

In this third case, there are three animations specified, but only two durations and iteration counts. In such cases where there are not enough values to give a separate value to each animation, the values cycle from start to finish. So for example, fadeInOut gets a duration of 2.5s and moveLeft300px gets a duration of 5s. We've now got to the end of the available duration values, so we start from the beginning again — bounce therefore gets a duration of 2.5s. The iteration counts (and any other property values you specify) will be assigned in the same way.
In this third example, three animations are specified, but only two durations and iteration counts. In such cases where there are not enough values in the list to assign a separate one to each animation, the value assignment cycles from the first to the last item in the available list and then resets to the first tem. So, `fadeInOut` gets a duration of `2.5s`, and `moveLeft300px` gets a duration of `5s`, which is the last value in the list of duration values. The duration value assignment now resets to the first value; `bounce`, therefore, gets a duration of `2.5s`. The iteration count values (and any other property values you specify) will be assigned in the same way.
Copy link
Member

@estelle estelle Aug 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In this third example, three animations are specified, but only two durations and iteration counts. In such cases where there are not enough values in the list to assign a separate one to each animation, the value assignment cycles from the first to the last item in the available list and then resets to the first tem. So, `fadeInOut` gets a duration of `2.5s`, and `moveLeft300px` gets a duration of `5s`, which is the last value in the list of duration values. The duration value assignment now resets to the first value; `bounce`, therefore, gets a duration of `2.5s`. The iteration count values (and any other property values you specify) will be assigned in the same way.
In this third example, three animations are specified, but only two durations and iteration counts. In such cases where there are not enough values in the list of durations to assign a separate one to each animation, the value assignment cycles from the first to the last item in the available list and then cycles back to the first item. So, `fadeInOut` gets a duration of `2.5s`, and `moveLeft300px` gets a duration of `5s`, which is the last value in the list of duration values. The duration value assignment now resets to the first value; `bounce`, therefore, gets a duration of `2.5s`. The iteration count values (and any other property values you specify) will be assigned in the same way.
If the mismatched length lists are inverted -- for example, if there are five `animation-duration` values for three `animation-name` values -- the extraneous animation property values, in this case, two `animation-duration` values, don't apply to any animation and are ignored.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to give this section and #### title, so we can link to it directly from the five animation pages where we have:

see Setting multiple animation property values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great suggestion. As I started working through it, I ended up pulling out "Using the animation shorthand" and "Setting multiple animation property values" sections from inside "Examples" and turned them from ### to ##.

I started to add ###s under "Setting multiple animation property values" as you've suggested but it seems two cases in the "Setting multiple animation property values" section are applicable in the other animation pages where we have that note. So it might not be worth adding the ###s and linking to just the last case.

BTW, thank you for suggesting the text for the inverse case 👍.

```

To learn more about the sequence in which different animation property values can be specified using the `animation` shorthand, see the {{cssxref("animation")}} reference page.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a warning to recommend putting the name last, since if a keyframe name is a keyword, the browser assumes the first instance is the keyword, not the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Text to this effect is present on the animation page, which is where the link is pointing.
(The sequence of sections on the 'animation' page does not seem consistent with other pages. I'll open another PR to fix that.)

@estelle
Copy link
Member

estelle commented Aug 25, 2022

Just three questions, then we're good to go.
I thought this had a glossary entry.... now I need to go find that to review.

@estelle
Copy link
Member

estelle commented Aug 26, 2022

I am on vacation next week. But my review isn't blocking

@estelle
Copy link
Member

estelle commented Aug 27, 2022

w3c/csswg-drafts#1594 provides an even better example.

@dipikabh
Copy link
Contributor Author

dipikabh commented Aug 29, 2022

Just three questions, then we're good to go. I thought this had a glossary entry.... now I need to go find that to review.

You'll find the glossary update in the other PR 🙂

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm not a CSS expert, not sure why I'm getting CSS review requests)

I skimmed over this and it looks good to me and it seems Estelle's comments are addressed, so approving. Feel free to merge (Estelle also said nothing blocking anymore).

@dipikabh dipikabh self-assigned this Aug 31, 2022
@bsmth
Copy link
Member

bsmth commented Sep 1, 2022

Going to merge this one now, thanks all 👍🏻

@bsmth bsmth merged commit 04b17a3 into mdn:main Sep 1, 2022
@dipikabh
Copy link
Contributor Author

dipikabh commented Sep 1, 2022

Thanks, @bsmth!

@dipikabh dipikabh deleted the anim-composition-related branch September 2, 2022 13:00
goshdarnheck pushed a commit to goshdarnheck/content that referenced this pull request Sep 7, 2022
* Adds link to animation-composition plus other edits

* Fixes errors

* Fixes review comments

* Fixes review comments

Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants