Page 1 of 1

2 pull requests

Posted: Wed Nov 29, 2017 11:43 am
by platywompus
Hey Sema,

I'm just starting to dive into your JZZ.js code for a piano learning project and I'm very impressed. I'll admit that even after a few days I'm still a novice, but I can appreciate the power and complexity in the architecture.

I have run into 2 minor problems that I was able to get around with some code tweaks and thought I'd share them. Before that, I'll admit that I may just be misunderstanding a concept so take these suggestions with a grain of salt :)

I've also submitted them via github, but wanted a chance to list out my thoughts in more depth.

1) possible loop conflict (.hasOwnProperty() tweak) pull req
Although I generally subscribe to the "simpler is better" js philosophy and avoid flashy ES6/node.js stuff in favor of just writing the damn code, my project has required I use some open source ES6 code (and have had to use babel to transpile it to ES5). This has required me to add the popular Array.prototype.find polyfill to get IE support (see and ... Array/find).

This works by adding a .find() prototype to all array-type objects, but it breaks in the JZZ.js code on the statements which are used regularly to execute chains of functions (now they try to execute the .prototype.find() method as well). By adding a .hasOwnProperty() check in those loops I was able to get JZZ.js to play nice with this prototype but still call all the user added method chains.

This change seems low risk and may help future proof JZZ.js as these sorts of polyfills and prototypes are becoming increasingly common.

2) closing and reopening ports pull req
One requirement i have is I want people to be able to open many pages of my learning site at once in different tabs (if they see multiple scores they want to practice on later) and have midi-in "just work" when they change tabs while practicing. Since the midi-in locks seem to be exclusive i wanted to be able to close ports on window.blur, then open them again on window.focus.

This tweak is small, and seems to work correctly, but I'm less sure about it as I haven't fully grasped the JZZ.js architecture. I found that currently closing then re-opening ports doesn't seem to work, but if I added logic to the port close function to also remove the port from the "_engine._inMap" array that they seem to support re-opening.

Though it seems to work and is a small tweak, it's possible I'm missing a greater implication, so i'm less sure you should accept this change.

Again, thanks for all the work! After spending a few days in the JZZ.js code base, the scope is humbling, but I appreciate it even more.



Re: 2 pull requests

Posted: Wed Nov 29, 2017 10:09 pm
by sema
Wow! Thanks a lot for your contribution!
The changes look reasonable, but I think I need a little more time to have a closer look...

Re: 2 pull requests

Posted: Thu Nov 30, 2017 8:03 pm
by sema
Done! Thanks again Michael!

Re: 2 pull requests

Posted: Fri Dec 01, 2017 8:37 am
by platywompus
Glad to help :D your tools (jzz, jazz-soft, jazz-midi) make my whole learning app possible! Its really encouraging to see projects like this pushing the limits of browsers.