Option to swap out requestMIDIAccess function when enabling WebMIDI #224
Conversation
|
Awesome! Thanks for the PR. Before I can merge it, there are a few things that need to be adjusted though...
My bad. I renamed the file when I moved from automatic generation to manual. I just committed it back. Are you able to run the tests now? Speaking of tests, we will need a unit test for that new option. It should go in the Also, can you copy the modified jsdoc comments to the |
jjeff
force-pushed
the
feat/requestMIDIAccessFunction
branch
2 times, most recently
from
a05002f to
48102e3
Compare
|
I just saw your updated PR moments ago. So sorry. I'm looking at it now. |
|
Wow! This is a mammoth PR. I will need more time to go through it. I'll get back to you. |
|
I probably should have commented after I submitted the changes. I actually thought I did... but it looks like I didn't. Sorry about that. Yeah. This changes all/most of the tests to use the new mock MIDI device. They're not all passing yet... and I'm not sure if it's the tests or the WebMidi library itself. But you would be better than me at determining that. Let me know if you have any questions. |
|
I don't know if I was supposed to receive a notification from GitHub in this scenario but I'm pretty sure I didn't. Anyway, I am SO sorry I missed your updates. The problem is that, meanwhile, I updated the library. So your PR rolls back some things I changed. Would it be at all possible for you to submit a new PR plotted against the latest version? Also, would it be possible to separate the changes to the tests from the changes to the library? I am a bit nervous to change the testing model, especially if some tests do not yes pass. Perhaps we could just mark the new Again, so sorry about the misunderstanding. I really wish to work with you on this as your PR will be very valuable to the community. |
|
Yeah. I'll take a look over the weekend and see if I can rebase on the current main and split the tests out into a separate PR. All good. 👍 |
adds a requestMIDIAccessFunction property to the options object argument of `WebMIDI.enable()`. If this property exists (and is a function), it will be called instead of `navigator.requestMIDIAccess`
jjeff
force-pushed
the
feat/requestMIDIAccessFunction
branch
from
48102e3 to
4462471
Compare
|
Okay. This PR has been rebased against main. Not a mammoth PR anymore... However, the Testing PR #232 is pretty mammoth and requires/includes this PR. That's where I could use some of your expertise in solving the testing problems. |
|
Awesome! Thank you so much @jjeff. |
2 participants
Adds a requestMIDIAccessFunction property to the options object argument of
WebMIDI.enable(). If this property exists (and is a function), it will be called instead ofnavigator.requestMIDIAccess. This seems to work in my tests, however I can’t get WebMIDI to build because there seems to be a missingtypescript-declarations/generate.jsfile… so I’m not sure if the unit tests are working correctly for me.