Feature/midi map #145
Feature/midi map #145
Conversation
There was a problem hiding this comment.
left a few inline comments. Additionally I noticed that the naming is a tad inconsistent as it uses at times midi and user times MIDI, should we just stick to the latter?
From a general UX perspective I'd be happy to tweak things a bit as I think it could be a more more obvious once one is in active "MIDIMapping" mode. Also I suspect we'd ideally like to disable changing parameter values from the UI when selecting a param to map and just maintain a "selection" state?
Another q would be - should we auto disable MIDI mapping when the user switches views / tabs or at least prevent direct navigation with a confirm dialogue?
ahh yeah, that is a good idea!
Yes, I was actually gonna ask about that, I briefly looked into it but then maybe i lost track |
I can just clean that up as part of polishing the UI a bit. Was thinking we could just follow the Max color scheme a bit here to rely on something already existing. Once u took a look at the suggestions just throw it my way and I can take a pass. |
|
the |
…y are not rendered when not "active", also removed duplicated "isMIDIMapping" prop from ParamTab which already has access to the instance itself
| @@ -315,7 +315,7 @@ export const clearParameterMidiMappingOnRemote = (id: InstanceStateRecord["id"], | |||
| if (!param) return; | |||
|
|
|||
| const meta = param.getParsedMeta(); | |||
| if (typeof meta === "object") { | |||
| if (typeof meta === "object" && !Array.isArray(meta)) { | |||
There was a problem hiding this comment.
fixes the TS build error as typeof [] is also an array. I guess the issue here is that AnyJSON means it could be any kind of value. I wonder if really what we want to ensure is that meta is a Record<string, AnyJSON> throughout and if not fall back to {}. Maybe even the Meta Editor should not allow any non JsonMap input as currently one could also provide [] which is valid JSON
There was a problem hiding this comment.
We have meta that we support but a user might have other ideas for it.. if they use midi mapping, osc mapping, they need to use objects but if they're simply adding meta that they use in their own way, i figure we shouldn't limit them?
There was a problem hiding this comment.
Hm, sure but the [param] object doesn't support it either - in fact doing anything than a Record<string, AnyJson> throws. Additionally setting it to eg [] means that functionality like MIDI mapping won't work so I personally am in favor of enforcing that it needs to be a JSON map at the top level.
|
ok tweaked things by
Also noticed that our parameter column layout was broken, so fixed that. Thing one issue is left if we really want to allow "any" JSON input in the meta editor or if it is safe to assume that |
| @@ -315,7 +315,7 @@ export const clearParameterMidiMappingOnRemote = (id: InstanceStateRecord["id"], | |||
| if (!param) return; | |||
|
|
|||
| const meta = param.getParsedMeta(); | |||
| if (typeof meta === "object") { | |||
| if (typeof meta === "object" && !Array.isArray(meta)) { | |||
There was a problem hiding this comment.
We have meta that we support but a user might have other ideas for it.. if they use midi mapping, osc mapping, they need to use objects but if they're simply adding meta that they use in their own way, i figure we shouldn't limit them?
|
Gonna break out the remaining tasks / discussions into tickets |
No description provided.