multichannel-to-equave implemented #649
Conversation
src/views/MidiView.vue
Outdated
| @@ -288,6 +303,15 @@ div.channels-wrapper span { | |||
| text-align: center; | |||
| } | |||
|
|
|||
| div.checkbox-group-scoped { | |||
There was a problem hiding this comment.
No need to name them scoped. Vue does the scoping for you.
|
|
src/views/MidiView.vue
Outdated
| </span> | ||
| </div> | ||
| <label>Settings for multichannel-to-equave mode</label> | ||
| <div class="control multichannel-input-scoped"> |
There was a problem hiding this comment.
It would be better UX if this div was collapsed or at least the inputs disabled when multichannel mode is off.
There was a problem hiding this comment.
how should I implement this? I agree but am not sure how to do it.
There was a problem hiding this comment.
I can do it. Just make an issue and assing it to me so that I don't forget. Same goes with checking all channels when changing modes.
|
I don't think the MIDI color mapping stuff makes sense with multichannel devices that don't have a piano layout. Maybe they should be ignored in multichannel mode. |
src/views/AboutView.vue
Outdated
| @@ -52,6 +52,7 @@ const tagline = computed(() => TAGLINES[Math.floor(Math.random() * TAGLINES.leng | |||
| Forrest Cahoon - <i>developer</i> <br /> | |||
| Videco - <i>developer</i> <br /> | |||
| Inthar - <i>developer</i> <br /> | |||
| Marc Sabat - <i>composer / developer</i> <br /> | |||
There was a problem hiding this comment.
Technically Scale Workshop does not ship with a soundtrack, so maybe notation advisor or hardware specialist instead of composer.
|
Overall seems good. Can't really test the functionality due to lack of hardware. |
|
working with my lumatone |
src/views/MidiView.vue
Outdated
| </div> | ||
| <label>Settings for multichannel-to-equave mode</label> | ||
| <div class="control multichannel-input"> | ||
| <span> |
There was a problem hiding this comment.
Semantically spans are for phrasing content. The styling on the input element puts it on a separate line so in terms of code style this should be a div.
src/views/MidiView.vue
Outdated
| type="number" | ||
| min="1" | ||
| max="16" | ||
| value="3" |
There was a problem hiding this comment.
Don't use value with v-model.
src/views/MidiView.vue
Outdated
| gap: 0.15rem 1rem; | ||
| } | ||
|
|
||
| div.multichannel-input { |
There was a problem hiding this comment.
Technically multichannel-input-container, but now I'm just splitting hairs.
|
Approved. Feel free to merge after fixing the minor issues. |
| @@ -4,6 +4,7 @@ import { Input, Output, WebMidi, type NoteMessageEvent, type MessageEvent } from | |||
| import MidiPiano from '@/components/MidiPiano.vue' | |||
| import { useMidiStore } from '@/stores/midi' | |||
| import { useScaleStore } from '@/stores/scale' | |||
| import { divNodes } from 'sonic-weave' | |||
|
When merging. Use the squash option to create a single commit with these changes. |
implemented changes suggested by frostburn and recoded over here as a feature branch. Hope I correctly updated to the current commits in main.