-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add the PointerEvent api #484
Conversation
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 really good, a few comments, but nothing big.
For API, we list every properties, methods, and eventually the constructor inside it (as "api"."PointerEvent".). This allows us to have a very easy way to add tables on methods/properties page.
Even if not all are listed I would approve the PR, but if you can it would be nice to list all of them (rather easy once the interface-level compat entry has been validated).
}, | ||
{ | ||
"version_added": "10", | ||
"prefix": "MS", |
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.
Lower case "ms". It is not case-sensitive and it is our convention (We wouldn't use MOZ or WEBKIT, but the lower case variants)
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 took it from here: https://developer.mozilla.org/en-US/docs/Glossary/Vendor_Prefix
Is that out of date? It specifically instructs uppercase for interfaces.
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.
Good point. Forget about my lowercase comments then.
api/PointerEvent.json
Outdated
"version_added": "55" | ||
}, | ||
"edge": { | ||
"version_added": true |
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.
"12"
api/PointerEvent.json
Outdated
"version_added": true | ||
}, | ||
"edge_mobile": { | ||
"version_added": true |
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.
"12"
}, | ||
{ | ||
"version_added": "10", | ||
"prefix": "MS", |
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.
Lower case.
api/PointerEvent.json
Outdated
"deprecated": false | ||
} | ||
}, | ||
"level-two-pen-attributes": { |
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.
Separate in two entries, one for each property.
@teoli2003 Do I also need to include the various related pointerCapture properties and methods that are on Element, not PointerEvent? |
Nope, only the ones on PointerEvent. The ones on Element (via the GlobalEventHandlers mixin or other). Will be added in an element.json when we do this one (yes it will be a complex one :-) ) |
Ok, well, I think this should be finished then :) |
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.
LGTM, r+
No description provided.