-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
Rewrite PerformanceEventTiming page #21531
Conversation
Preview URLs Flaws (11)URL:
(this comment was updated 2022-10-19 09:52:19.675943) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is much better Florian.
I agree that the table is better than the list.
I haven't finished reviewing yet: will have a closer look at the examples in a bit.
Co-authored-by: wbamberg <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Florian! I think I am done reviewing this now. You should probably manually wrap the comment at line 234:
// Create a PerformanceObserver that calls `onFirstInputEntry` for each entry.
...or Yari will wrap it for you :(.
@@ -15,56 +15,145 @@ browser-compat: api.PerformanceEventTiming | |||
|
|||
{{APIRef}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{{APIRef}} | |
{{APIRef("Event Timing API"}} |
I think? I hate our WebAPI sidebars and IA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have a groupData thing for this. We should probably figure out what to put here for all performance APIs
Co-authored-by: wbamberg <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thank you Florian!
Description
This PR rewrites the
PerformanceEventTiming
interface page.I think the list of exposed events needs to be grouped somehow. At least I don't like how it is just a plain list right now. It just looks very lost (see what's currently live: https://developer.mozilla.org/en-US/docs/Web/API/PerformanceEventTiming). So, I made it an HTML table. Not sure if we generally want to avoid HTML tables, but I think this is somewhat tabular data and to my eyes it reads and scans a lot better this way. Open to do it differently, though.
I then added a bit of an intro text to the interface so people can understand what problem this API solves.
There is no constructor, but I always like to give people an idea how to get to the information an interface holds.
I also simplified the first example a bit to be very pure, so it can act as minimal boilerplate code for working with this interface. I think the second example also looks a bit packed, especially because since MDN uses a 3-column layout it doesn't have much space to breathe. I'm not sure what to do about it, though.
Finally, I updated the list of instance properties, one was missing and they are all read-only.
Motivation
The Performance API docs need to be improved! See openwebdocs/project#62.
Additional details
Will have follow-up PRs for the sub pages.
Related issues and pull requests
None.