Renovated web interface and modern improvements#5
Renovated web interface and modern improvements#5aldipower wants to merge 18 commits intowherget:masterfrom
Conversation
|
Hi Felix! Wow, the picture is already mighty impressive. 😲 Thank you very much for your contribution. I'll go have a thorough look through the gorgeous frontend you built. I'm happy to have my previous proof-of-concept-y rudiments thrown away, I just want to familiarize myself with it before I land this. Can't quite give you an ETA right now, though I'm hopeful I'll find some time around the holidays at the latest. Again, thanks a lot! |
|
Hi, that sounds great. If you have any questions feel free to mention it in this PR of course, I will try to explain then. |
|
I've forgotten to mention, I couldn't test it with an iPad/Safari. Maybe you have one? It should work out-of-the-box, but sometimes the Safari browser reveals some surprises... |
|
I've tested it on an older android tablet now with an old Chrome version, that turned out to be unsuccessful, so I've added the following section to the README. There would be probably a solution I could figure out, but taking our "huge" user base into consideration, I think this is ok to let someone install a newer browser. :-) CompabilityFor mobile devices you need at least Chrome 140, Firefox Mobile or a recent Safari for iOS. The web ui should work on every modern desktop browser though. |
|
I finally got around to do a little bit of testing and can confirm multitouch works as expected even on an older iPad (iPadOS 18.5). I'm planning to do the hardware checks with the mixer next week. Let's see if I get around to do some code review until then. |
wherget
left a comment
wherget
left a comment
There was a problem hiding this comment.
I've left some code review comments. By no means do I expect you to all fix them by yourself, they're just as well notes on what I would improve myself before I want to land it. If you have the time, please let me know what you think.
| .fine-tune .minus-btn { | ||
| margin-left: 8px; | ||
| } | ||
| .fine-tune .plus-btn { | ||
| margin-right: 8px; | ||
| } |
There was a problem hiding this comment.
The margins could be even smaller to have a larger area available for touch inputs. Maybe 3px? A little separation is necessary so a tap on plus doesn't accidentally land on minus for the neighboring channel.
| .fine-tune button { | ||
| flex-grow: 1; | ||
| background-color: transparent; | ||
| padding-top: 12px; | ||
| padding-bottom: 12px; | ||
| margin-bottom: -8px; | ||
| border-radius: 6px; | ||
| border: none; | ||
| color: #eee; | ||
| font-size: 12px; | ||
| cursor: pointer | ||
| } |
There was a problem hiding this comment.
I think a little (very faint!) affordance would be nice to see that these buttons are actually active elements. Just adding the border or background back looks messy because of the buttons' size (which is good for an iceberg-style touch control) though. Might need to experiment a bit.
| </div> | ||
| <div class="fader-right-ticks"></div> | ||
| </div> | ||
| <div class="fader-mouse-overlay"></div> |
There was a problem hiding this comment.
This area could expand a bit further to the top (maybe up to and including the dB value) and more importantly the bottom (at least expanding by half the fader knobs size into the margin of the secondary function).
Mainly I would love it to be even easier to "start grabbing" a fader at -inf, as the touch start event may not register if you dont "grab the top half" of the knob graphic. Laying a finger smack dab in the middle of the skeumorphic fader groove (at -inf) might be cutting it close already.
(Wow, the amount of polish you put in here makes me really aim for perfection even more. 😅)
| fader.querySelector(".colorbar.bottom").style.background = color; | ||
| } | ||
|
|
||
| const faderName = localStorage.getItem("faderName" + faderIndex) || ""; |
There was a problem hiding this comment.
Here we could add some predefined fader names, which would be especially helpful for Bus and Aux and FX Channels.
| } | ||
|
|
||
| Channel.prototype.setPan = function(dB) { | ||
| this.pan_db = dB; |
There was a problem hiding this comment.
I know the SysEx manual calls it "dB" but I find it misleading as it's just 0 to 60 with 30 presumably being center?
AFAICS this currently should be called with values from -30 to +30.
I'm not enforcing inputs with other parameters, but a comment and name change might be helpful for future developers (i.e. probably future me).
| function sysExPanToNormalized(sysExValue) { | ||
| return sysExValue / 60 | ||
| } | ||
|
|
||
| function normalizedPanToSysEx(value) { | ||
| return Math.round(value * 60); | ||
| } |
There was a problem hiding this comment.
I think I see where normalized meaning 0 to 1 comes from (I bet it's because of the relative offsets for the pointer inputs and positioning of the fader and secondary bar). It sure doesn't help that midi is passed through straight to the frontend, such that 0 to 60 also crops up here. Unfortunately that means there's conversions to -30 to 30 for the text display and submission back to the backend (oh my!) too.
General advice, my gut feeling is that since "normalized" is overloaded in the audio world already, percentage might be a better prefix to indicate what the expected return value is.
That aside, it feels to me that all this conversion could use some pulling together to a central place, maybe a class that encapsulates the SocketIO and midi conversion stuff for the frontend?
| redraw, | ||
| updateSecFader, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Wow is createFader a long function. Again, maybe this could benefit from the cached values (what is currently secValues) moving out into an object. Needn't be the same one that handles SocketIO and MIDI that I talked about earlier, but might be related.
My main gripe is that I'm having a hard time identifying the lines where the actual mapping from UI state to remote and back occurs. Theres all the setX functions, changeSecValue is relatively condensed, but then handleMuteButtonClick sneaks in a conversion from "element has class". My "separation of concerns" sense is tingling is probably all.
| sockIO.emit(type, | ||
| JSON.stringify({ | ||
| "channel": faderIndex + 1, | ||
| "parameter": currentSecType.substring(type.length), |
There was a problem hiding this comment.
Oooh, that is a tight coupling of UI text with functionality. Especially as a global switch. My "stringly typed" antipattern sensor is itching.
Maybe the "secondary type" switching logic could do with a little extraction and polish. Maybe just have currentSecondary = {type: "aux", parameter: 4 } as an object and cycle through a list of those?
|
Hi! |
2 participants
Hi! I hope you are doing well.
DDX owners are getting rare nowadays. :-)
I've took your project and renovated it, so I can use it on my tablet besides my drum machine controlling the DDX at the other side of the room.
Hope you like it and merge it upstream. Maybe some (or one?) others will find it useful too. And thanks for your initial work.
All the best from Hamburg.
Features
Ein Bild sagt mehr als tausend Worte:
