-
Notifications
You must be signed in to change notification settings - Fork 49
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
Integrate with Permissions #219
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.
good stuff... and you are right, the whole .requestMIDIAccess()
section be replaced by hooking into our spec.
However, seems "software" as type got added at some point.
requestMIDIAccess(optional MIDIOptions options = {});
should be changed to:
requestMIDIAccess(optional MidiPermissionDescriptor options = {});
And MidiPermissionDescriptor
should gain the software
member (we can copy/pasta the definition that's already there for MidiOptions
). I think MidiOptions
can then be deleted form the spec.
(we can do the above in two passes...) |
Co-authored-by: Marcos Cáceres <[email protected]>
Yeah, let me file an issue to do that as a follow up. PTAL @cwilso! |
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.
Thanks Mike! |
Thanks for the review! |
As part of w3c/permissions#263, this PR moves some of the Permissions stuff into WebMidi (so Permissions doesn't have to be a registry, but instead defines powerful features infrastructure).
It also fixes some bugs in the Permissions policy section.
cc @marcoscaceres