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

feat: Editorial review: Long animation frames API docs #33039

Merged
merged 41 commits into from
Apr 19, 2024

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented Apr 11, 2024

Description

#32937 contains the engineering technical review for my work on Long Animation Frames API docs, which has been completed and approved. Thank you to @tunetheweb and @noamr for your thorough and detailed review work.

This is a new PR based on the same branch, which is intended to contain the editorial review for the same work.


Background information

The Long Animation Frames API was released in Chrome 123. This PR adds docs for this API.

Specifically, it:

  • Adds a guide to using the API
  • Adds reference docs for the PerformanceLongAnimationFrameTiming interface.
  • Adds reference docs for the PerformanceScriptTiming interface.
  • Adds entries to existing docs that require them, such as https://developer.mozilla.org/en-US/docs/Web/API/PerformanceEntry/entryType
  • Updates GroupData.json so that the relevant new items appear in the Performance API sidebar

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested review from a team as code owners April 11, 2024 11:58
@chrisdavidmills chrisdavidmills requested review from wbamberg and dipikabh and removed request for a team April 11, 2024 11:58
@github-actions github-actions bot added Content:WebAPI Web API docs size/xl [PR only] >1000 LoC changed labels Apr 11, 2024
Copy link
Contributor

github-actions bot commented Apr 11, 2024

Preview URLs (24 pages)
Flaws (1)

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

URL: /en-US/docs/Web/API/PerformanceScriptTiming/sourceFunctionName
Title: PerformanceScriptTiming: sourceFunctionName property
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: api.PerformanceScriptTiming.sourceFunctionName
External URLs (10)

URL: /en-US/docs/Web/API/PerformanceScriptTiming
Title: PerformanceScriptTiming


URL: /en-US/docs/Web/API/PerformanceScriptTiming/forcedStyleAndLayoutDuration
Title: PerformanceScriptTiming: forcedStyleAndLayoutDuration property


URL: /en-US/docs/Web/API/Performance_API/Long_animation_frame_timing
Title: Long animation frame timing


URL: /en-US/docs/Web/API/PerformanceLongAnimationFrameTiming/blockingDuration
Title: PerformanceLongAnimationFrameTiming: blockingDuration property

(comment last updated: 2024-04-19 14:49:13)

@chrisdavidmills chrisdavidmills changed the title Long animation frames api feat: Editorial review: Long animation frames API docs Apr 11, 2024
@wbamberg wbamberg removed their request for review April 11, 2024 15:36
Copy link
Contributor

@skyclouds2001 skyclouds2001 left a comment

Choose a reason for hiding this comment

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

typo - readonly => read-only

Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Apr 12, 2024
Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

I've looked at all the pages now except https://pr33039.content.dev.mdn.mozit.cloud/en-US/docs/Web/API/Performance_API/Long_animation_frame_timing. I'll be able to get to it tomorrow.

Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

I think I've looked at all the pages in this PR now.


### Long Animation Frames API feature detection

You can test whether the Long Animation Frames API is supported using {{domxref("PerformanceObserver.supportedEntryTypes")}}:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can test whether the Long Animation Frames API is supported using {{domxref("PerformanceObserver.supportedEntryTypes")}}:
You can test whether the Long Animation Frames API is supported using {{domxref("PerformanceObserver.supportedEntryTypes_static")}}:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this change is needed. The macro is already clever enough to resolve to the correct URL, and if you explicitly add this, it appears in the link text, which is not what we want (it's not part of the actual property name).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the flaw does say it will redirect:

Explanation: /en-US/docs/Web/API/PerformanceObserver/supportedEntryTypes redirects to /en-US/docs/Web/API/PerformanceObserver/supportedEntryTypes_static

It also says that the flaw is fixable.

If you want, it could be updated to:

{{domxref("PerformanceObserver.supportedEntryTypes_static", "PerformanceObserver.supportedEntryTypes")}}

...the way it's been done on getEntriesByType.

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner April 19, 2024 11:52
@chrisdavidmills chrisdavidmills requested review from hamishwillee and removed request for a team April 19, 2024 11:52
@github-actions github-actions bot added the Content:Glossary Glossary entries label Apr 19, 2024
---
title: Long animation frame timing
slug: Web/API/Performance_API/Long_animation_frame_timing
page-type: web-api-overview
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a guide page-type? Elsewhere, we've used web-api-overview on API landing pages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol, yup! That was a copy-paste error. Fixed now.


### Long Animation Frames API feature detection

You can test whether the Long Animation Frames API is supported using {{domxref("PerformanceObserver.supportedEntryTypes")}}:
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the flaw does say it will redirect:

Explanation: /en-US/docs/Web/API/PerformanceObserver/supportedEntryTypes redirects to /en-US/docs/Web/API/PerformanceObserver/supportedEntryTypes_static

It also says that the flaw is fixable.

If you want, it could be updated to:

{{domxref("PerformanceObserver.supportedEntryTypes_static", "PerformanceObserver.supportedEntryTypes")}}

...the way it's been done on getEntriesByType.

## See also

- [Optimize long tasks](https://web.dev/articles/optimize-long-tasks) on web.dev (2024)
- [Where long tasks fall short](https://github.com/w3c/long-animation-frames#where-long-tasks-fall-short), Long Animation Frames API explainer (2024)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

Suggested change
- [Where long tasks fall short](https://github.com/w3c/long-animation-frames#where-long-tasks-fall-short), Long Animation Frames API explainer (2024)
- [Where long tasks fall short](https://github.com/w3c/long-animation-frames#where-long-tasks-fall-short) on W3C (2024)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dunno; I would expect a link labeled "W3C" to go somewhere on W3.org.

observer.observe({ type: "long-animation-frame", buffered: true });
```

## Comparison with the Long Tasks API
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Comparison with the Long Tasks API
## Comparing with the Long Tasks API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, updated


## Comparison with the Long Tasks API

The Long Animation Frames API was preceded by the [Long Tasks API](https://w3c.github.io/longtasks/) (see {{domxref("PerformanceLongTaskTiming")}}) — these specs both have a similar purpose and usage, exposing information on [long tasks](/en-US/docs/Glossary/Long_task) that block the main thread for 50ms or more.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we mean the functionalities that these specs provide

Suggested change
The Long Animation Frames API was preceded by the [Long Tasks API](https://w3c.github.io/longtasks/) (see {{domxref("PerformanceLongTaskTiming")}}) — these specs both have a similar purpose and usage, exposing information on [long tasks](/en-US/docs/Glossary/Long_task) that block the main thread for 50ms or more.
The Long Animation Frames API was preceded by the [Long Tasks API](https://w3c.github.io/longtasks/) (see {{domxref("PerformanceLongTaskTiming")}}) — both the APIs have a similar purpose and usage, exposing information about [long tasks](/en-US/docs/Glossary/Long_task) that block the main thread for 50ms or more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Thanks for addressing all my comments, @chrisdavidmills! I'm leaving a +1 with just a few minor things for you to go over.

Appreciate your work on adding all these pages, thank you!

@chrisdavidmills
Copy link
Contributor Author

Thanks for addressing all my comments, @chrisdavidmills! I'm leaving a +1 with just a few minor things for you to go over.

Appreciate your work on adding all these pages, thank you!

@dipikabh super awesome, thanks for another great review!

That's everything addressed, so I should be able to merge soon.

@chrisdavidmills chrisdavidmills merged commit 1391eec into mdn:main Apr 19, 2024
9 checks passed
@chrisdavidmills chrisdavidmills deleted the long-animation-frames-api branch April 19, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Glossary Glossary entries Content:WebAPI Web API docs size/xl [PR only] >1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants