nicroto
commented
7 years ago Hi,
Sorry for the late response. And thanks for working on this. But I'm afraid I can't merge the pull request. Mainly because I don't agree with at least couple of things about it.
That being said, I think we can fix those issues and merge the bulk of your work.
For future contributions, that you might be doing on the NV-1, please remember to always trigger a discussion with me before writing any code. As I've said in the README: "Always submit an issue (and wait for response from an owner), before you start coding, except in obvious situations like non-breaking fixes;"
You should've commented on the issue (since there is one already), so we can decide on what exactly needs to be done there.
Now on the problems at hand.
Too much text.
My main problem is that all of these fields, visible at all times... that doesn't look good to me. It's too much text. If you open the official version, right beside it - you'll see it even stretches the background.
When you think about it, when do you even need such finer control of a parameter - you would only need it on params with huge amount of potential values, where precision of hand movement, just doesn't cut it. So:
- waveforms - not a good candidate;
- any dial or slider with the exact values drawn around it - not a good candidate;
- pitch-bend - not a good candidate (it's a tool for live-play and it should return to middle after change);
- mod-wheel - not a good candidate (it's a tool for live-play not pretuned settings);
- dials and sliders with no values around them - an ideal candidate.
Still, those are quite a lot:
- the entire effects section;
- all volumes;
- voice count;
- glide;
- all envelope sliders;
- etc.
The best thing that I can think of is showing the text field only if you start tweaking the parameter. Maybe not even on hover, but when you have made a full click (like selecting the control). Then you see a textbox which you can change. And clicking somewhere else hides it and so does a timeout after you last used it (it will autohide unless you do something with it within certain time). And it shouldn't push any content either (absolute container on top of it).
Implementation.
You've implemented it directly into my unofficial version of the webaudio-controls, which is inside of the repo. It may not be the best idea to change the project and make it diverge any further from the official one (I think they even merged my changes since, just haven't had the time to do maintenance on this), but I am kind of OK of doing it this way, since if anything else, at some point I want to remove that dependency all together and use something else.
Here the blame is partially mine - I've included the code of webaudio-controls, because I needed to make some fixes which I wasn't sure when (if at all) are going to land in the official project's repo. But I do have a fork in my account.
So, the procedure would be to fork my fork of webaudio-controls, add your changes there (diff the implementation in the NV-1 with each branch to see which one was holding the current state, if it isn't master), submit a pull request. I would look at that and merge the changes in my fork of webaudio-controls and update the NV-1 repo with the new version.
Otherwise I like the non-invasive style of your changes as footprint in the NV-1's repository (haven't had the time to revise the actual changes to webaudio-controls).
What can we do to preserve your work?
Here are the steps that I see:
- Lets finish the design of the feature with a couple of mugshots (I can draw these and will) and do this at the thread of the github issue.
- Then you can implement these changes in your fork of NV-1 (the one you made the pull request from).
- You can continue updating the public demo, so I can verify the feature is done.
- Having implemented the final feature, you can first fork webaudio-controls from me (I'll provide more info on the exact repo and branch in the discussion under the github issue) and transfer your changes from your NV-1 fork there and make a pull request;
- I will merge the webaudio-controls pull request (we may need to do a few iterations if there is something that doesn't look good as implementation there);
- I will update the NV-1's copy of it;
- You will delete your changes to webaudio-controls and commit to master of your NV-1 fork;
- You will pull latest master of NV-1 to get the webaudio-controls;
- And will finally issue a new pull request to the NV-1 repo.
That's the only way I see to get this done right and not to lose your changes.
Please note - I am only willing to even just discuss this complex fix, because it's the first time that someone has decided to contribute to the project. But now you know - first we discuss what needs to be done, than we work on it, not the other way around. This is the only way any future contributions will get in.
Let me know, if you want to do this in the github issue for the task.
Happy Holidays!
taylorstutts
Added text inputs to webaudio knobs and sliders. This is implemented in the embedded CSS and JS in the webaudio knob and slider HTML files, the template files for the various components, and their respective style files. Currently live demo at https://taylorstutts.github.io/viktor/ .