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.

Sign up for GitHub

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

Jump to bottom

Update event features according new data guideline #14578

Closed
Elchi3 opened this issue Jan 18, 2022 · 8 comments
Closed

Update event features according new data guideline #14578

Elchi3 opened this issue Jan 18, 2022 · 8 comments
Labels
bulk_update 📦 An update to a mass amount of data, or scripts/linters related to such changes good first issue 💯 Good issues for getting started with this project. help wanted 🆘 You're encouraged to pick up this issue, a maintainer will come back to you and review your work.

Comments

@Elchi3
Copy link
Member

Elchi3 commented Jan 18, 2022 •
edited by queengooborg
Loading

The data guideline for how to represent events in BCD has been updated recently. See https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines.md#dom-events-eventname_event for the new guideline.

This is a tracking issue to update all event features to align with the said guidelines. Practically, this means that there shouldn't be any on event handler features. We can grep them: npm run traverse all api|grep "\.on".

If mixins, bubbling events, or global events are involved, it might not be straightforward what to do but usually, the on event features are merged with the eventName_event features, making two BCD entries one entry.

All changes usually require an MDN content PR as well. Use the "needs content update" label and ideally also submit the MDN PR that changes the content structure. The MDN contribution docs contain a template that shows what new event pages should look like. Example page: https://developer.mozilla.org/en-US/docs/Web/API/XRSession/end_event

Help and expertise around events are appreciated.
@queengooborg queengooborg added bulk_update 📦 An update to a mass amount of data, or scripts/linters related to such changes help wanted 🆘 You're encouraged to pick up this issue, a maintainer will come back to you and review your work. good first issue 💯 Good issues for getting started with this project. labels Jan 18, 2022
@teoli2003 teoli2003 mentioned this issue Feb 9, 2022
Merged
@Elchi3 Elchi3 mentioned this issue Feb 9, 2022
Closed
@teoli2003 teoli2003 mentioned this issue Feb 9, 2022
Merged
@queengooborg queengooborg pinned this issue Feb 10, 2022
@teoli2003 teoli2003 mentioned this issue Feb 10, 2022
Merged
This was referenced Feb 11, 2022
Merged
Merged
Merged
Merged
Merged
Merged
@teoli2003 teoli2003 mentioned this issue Feb 14, 2022
Merged
@teoli2003
Copy link
Contributor

teoli2003 commented Feb 14, 2022 •
edited
Loading

I'm working on checklists so we can do this with less errors! Here they are:
Warning: WORK IN PROGRESS– DO NOT TRUST THIS YET – I'll remove this comment when you can

AWhen onXYZ exists and XYZ_event not, for each onXYZ

  • yarn content move /Web/API/xxxx/onXYZ /Web/API/xxxx/XYZ_event
  • Update browser-compat front-runner key
  • Update title front-runner key to: Interface: XYZ event (with ticks!)
  • Remove Property and Event Handler from the tag list and add Event instead (more cleaning possible)
  • Check if the event bubbles. Do nothing if not.
  • Change intro to explain the event itself and no more just onXYZ.

    Example: "The levelchange event of the Battery Status API is fired when the battery {{domxref("BatteryManager.level","level")}} property is updated."

  • Copy the syntax section from the template, and fix the names of event and of the onXYZ property
  • Add the Event type section (two texts possible! If inheritance is not only Event, do these additional steps:
    • Update the argument of {{InheritanceDiagram}}
    • Add the Event properties section and list the parameters, taken from the interface of the event type
  • Check that no mention of this onXYZ is a link.
  • Check that links to onXYZ is a link to the adequate XYZ_event

When both onXYZ and XYZ_event exist:

  • Move any relevant info from onXYZ to XYZ_eventpage
  • Remove Property and Event Handler from the tag list and add Event instead (more cleaning possible)
  • Check if the event bubbles. Do nothing if not.
  • Change intro to explain the event itself and no more just onXYZ (it may be already ok).

    Example: "The levelchange event of the Battery Status API is fired when the battery {{domxref("BatteryManager.level","level")}} property is updated."

  • Copy the syntax section from the template, and fix the names of event and of the onXYZ property
  • Add the Event type section (two texts possible! If inheritance is not only Event, do these additional steps:
    • Update the argument of {{InheritanceDiagram}}
    • Add the Event properties section and list the parameters, taken from the interface of the event type
  • Check that no mention of this onXYZ is a link.
  • Check that links to onXYZ is a link to the adequate XYZ_event
  • yarn content delete /Web/API/xxxx/onXYZ --redirect /Web/API/xxxx/XYZ_event

On the API interface page:

  • If existing, remove the Event handlers section
  • If not there, add the Events section
  • Check that there are no links to onXYZ.

On each page of the interface and the API overview page:

  • Check that there are no links to onXYZ.
  • Check that examples uses the modern syntax for defining function in event handler (onXYZ = event => { …} )
  • Check that no {{event} macro is left.
  • Fix flaws (as much as possible); at least, make sure not to introduce new flaws.
  • Check that if an event is linked to a property value, that property documents when this event is fired.

    Example: "When its value changes, the chargingchange event is fired."

Optional:

  • Remove gremlins (non-breaking spaces, curly tick, curly double-ticks)
  • If the interface is no more experimental, remove the experimental tags/banners.

Administration:

  • Link to the BCD PR.

@ddbeck
Copy link
Collaborator

ddbeck commented Feb 15, 2022 •
edited by queengooborg
Loading

(Both of these are WIP; feel free to edit.)

A checklist for creating an event BCD PR (for authors):

  • Confirm existence or create _event feature (follow the guideline)
  • Reconcile data in on event handler feature with _event feature (e.g., copy relevant notes over)
  • Remove on feature
  • Set mdn_url for _event feature
  • Set spec_url for _event feature
  • Set descriptionfor _eventfeature
  • Run npm run fix to fix sorting, if needed
  • Run npm test
  • Open PR, including a link to or copy of the reviewer checklist below
  • Comment or edit PR description with a link to _event feature source if it's not in the diff

A checklist for reviewing an event BCD PR (for reviewers):

  • Confirm removal of features for event handlers starting with on (as in onclick)
  • Make sure the feature ending in _event exists (as in click_event)
  • Make sure the feature ending in _event comports with the data in the removed handler (e.g., notes needed have been carried over)
  • Confirm _event mdn_url points to the updated page
  • If applicable, confirm _event spec_url
  • Before merging, confirm that content PR is merged (or at least open and close to being merged)

@teoli2003 teoli2003 mentioned this issue Feb 15, 2022
Merged
10 tasks
This was referenced Feb 16, 2022
Merged
Merged
@teoli2003 teoli2003 mentioned this issue Feb 16, 2022
Merged
10 tasks
queengooborg added a commit to queengooborg/browser-compat-data that referenced this issue Feb 17, 2022
@queengooborg
Adapt BaseAudioContext API to new events structure
12dc824 
This PR adapts the BaseAudioContext API to conform to the new events structure.  Part of work for mdn#14578.
This was referenced Feb 17, 2022
Merged
Merged
queengooborg added a commit to queengooborg/browser-compat-data that referenced this issue Feb 18, 2022
@queengooborg
Adapt AudioTrackList API to new events structure
956bbdb 
This PR adapts the AudioTrackList API to conform to the new events structure.  Part of work for mdn#14578.
@queengooborg queengooborg mentioned this issue Feb 19, 2022
Merged
queengooborg added a commit to queengooborg/browser-compat-data that referenced this issue Feb 19, 2022
@queengooborg
Adapt api.AudioWorkletNode.processorerror_event to new events structure
d56e018 
This PR adapts the processorerror event of the AudioWorkletNode API to conform to the new events structure.  Part of work for mdn#14578.
queengooborg added a commit to queengooborg/browser-compat-data that referenced this issue Feb 19, 2022
@queengooborg
Adapt DedicatedWorkerGlobalScope API to new events structure
0d4e822 
This PR adapts the DedicatedWorkerGlobalScope API to conform to the new events structure.  Part of work for mdn#14578.
@queengooborg queengooborg mentioned this issue Feb 19, 2022
Merged
Elchi3 pushed a commit that referenced this issue Feb 21, 2022
@queengooborg
Adapt AudioTrackList API to new events structure (#15029)
433cdf9 
This PR adapts the AudioTrackList API to conform to the new events structure.  Part of work for #14578.
Elchi3 pushed a commit that referenced this issue Feb 21, 2022
@queengooborg
Adapt BaseAudioContext API to new events structure (#15019)
c656748 
This PR adapts the BaseAudioContext API to conform to the new events structure.  Part of work for #14578.
Elchi3 pushed a commit that referenced this issue Feb 21, 2022
@queengooborg
Adapt api.AudioWorkletNode.processorerror_event to new events structu…
114ab7a 
…re (#15044)

This PR adapts the processorerror event of the AudioWorkletNode API to conform to the new events structure.  Part of work for #14578.
@queengooborg queengooborg mentioned this issue Mar 7, 2022
Closed
@Rumyra Rumyra mentioned this issue Mar 10, 2022
Merged
@queengooborg
Copy link
Collaborator

Hey @teoli2003 @Rumyra! Quick question: are you two still planning on tackling the events marked as "in progress"? I'd be happy to review PRs for them if so, or take them on if not!

This was referenced Mar 18, 2022
Merged
Merged
Merged
@Rumyra
Copy link
Collaborator

Rumyra commented Mar 18, 2022

Hey @queengooborg - see the above linked prs. I am very sorry for doing MIDIAccess & MIDIPort together in the content one, but together they touched the same pages & were the same event type (I wasn't planning to - it just happened 😳 )

Hope I got the spec url move in the bcd ones too - let me know 👍

@queengooborg
Copy link
Collaborator

Thanks @Rumyra! No worries about doing those two interfaces together, especially since they're related interfaces and members!

ddbeck pushed a commit that referenced this issue Mar 22, 2022
@queengooborg
Adapt Worker/Message API to new events structure (#15046)
ffa5f37 
* Adapt DedicatedWorkerGlobalScope API to new events structure

This PR adapts the DedicatedWorkerGlobalScope API to conform to the new events structure.  Part of work for #14578.

* Apply updates to related APIs

* Add messageerror_event to ServiceWorkerContainer

* Fix sorting
@teoli2003
Copy link
Contributor

Hey @teoli2003 @Rumyra! Quick question: are you two still planning on tackling the events marked as "in progress"? I'd be happy to review PRs for them if so, or take them on if not!

Hey @queengooborg Feel free to take over mine (I removed my name from them)

@Elchi3
Copy link
Member Author

Elchi3 commented Apr 28, 2022

Everything got done here except GlobalEventHandlers. It's unclear how to resolve it.

See the discussions:
#15480
#7545 (comment) ff.
#16029
#16026

@queengooborg queengooborg mentioned this issue Jun 5, 2022
Closed
@queengooborg
Copy link
Collaborator

queengooborg commented Jul 25, 2022 •
edited
Loading

Great news: we have now finished demixing every event handler in GlobalEventHandlers, which marks the completion of this issue! 🎉

I realize that this issue was never updated to indicate that we made a decision about GlobalEventHandlers. There has been lots of conversation across many places, but the main decision was made during a BCD meeting. Philip and I decided that it would be best to demix GEH for two main reasons: it is the only remaining mixin in BCD that hasn't been moved to _mixins, and each event has a more complicated history than just "supported on all these elements" that is obfuscated by placing it all on GEH. As such, we have moved ahead with demixing it.

@queengooborg queengooborg unpinned this issue Jul 25, 2022
@queengooborg queengooborg mentioned this issue Jul 25, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bulk_update 📦 An update to a mass amount of data, or scripts/linters related to such changes good first issue 💯 Good issues for getting started with this project. help wanted 🆘 You're encouraged to pick up this issue, a maintainer will come back to you and review your work.
Projects
MDN Contributors Task Board
Status: Done
Milestone
No milestone
Development

No branches or pull requests
5 participants
@ddbeck @Rumyra @Elchi3 @teoli2003 @queengooborg