Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI Support for global SetViews #192

Draft
wants to merge 16 commits into
base: develop
Choose a base branch
from
Draft

UI Support for global SetViews #192

wants to merge 16 commits into from

Conversation

fde31
Copy link
Member

@fde31 fde31 commented Dec 20, 2024

This adds UI support for global, user defined parameters views in a set. A few things we might want to discuss probably:

Naming

Currently everything uses SetView in order to refer to this new paradigm. Not sure that's what we to use in the end, it seems a bit too code-y, especially given the camelCase?

SetView Management

I've moved the SetView management to a drawer, basically following the UX of presets and GraphSets. I think one aspect that is still missing from the UI currently is implementing sort_order.

Screenshot 2024-12-20 at 19 13 03

Parameter Ordering / Management

I've moved param ordering and management into a dropdown and a modal for now. Would be great to get DnD in the list view in place (and maybe in other places of the app too) but thought we might not want to hold back the feature bc of that and could instead ticket as a follow up FR?

Also the modal UI is relatively primitive but if we want to change that or if it requires a some redesigning it shouldn't be too hard. Tbh would prefer a diff set of eyes or some user feedback here to determine what would be helpful.

Screenshot 2024-12-20 at 19 13 36

image

View support for sets

Currently it's a bit confusing that one can be in a state that is considered "No Set" by the runner which also means that there is no support for SetViews. Maybe related to #182 but I wonder what your thoughts are about this and whether it should actually be possible to be in a "no set state" moving forward?

Setting to draft for now.

See #179

fde31 added 16 commits December 18, 2024 13:57
no paraameter selection, rendering, interaction yet but provides shell for creating and editing of views
…pport passing the actual ComponentType for the ParameterItem as well as extra props
… passed, if the action actually needs it one can retrieve it from the state quite easily given param.instanceIndex
… item and moved it to withMidiMapping Component while extending SetView Parameters with basic ordering and SetView removal menu
@fde31 fde31 requested a review from x37v December 20, 2024 19:09
Copy link
Contributor

@x37v x37v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking great. A couple of little notes.
really happy that we can see if set view params are MIDI mapped, I'm sure folks are going to want to MIDI map from there so maybe we should create a ticket for that?
Seems like at least metadata editing should be easy to add right there?

type: GraphSetActionType.INIT_SET_VIEWS,
payload: {
views
// names.map(n => PresetRecord.fromDescription(n, n === "initial"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cruft

Comment on lines +472 to +519
export const decreaseParameterIndexInSetView = (setView: GraphSetViewRecord, param: ParameterRecord): AppThunk =>
(dispatch) => {
try {
const currentIndex = setView.params.findIndex(entry => entry.instanceIndex === param.instanceIndex && entry.paramIndex === param.index);
if (currentIndex <= 0) return;

const newList = setView.params
.delete(currentIndex)
.insert(currentIndex - 1, { instanceIndex: param.instanceIndex, paramIndex: param.index });
const message = {
address: `/rnbo/inst/control/sets/views/list/${setView.id}/params`,
args: newList.toArray().map(p => ({ type: "s", value: instanceAndParamIndicesToSetViewEntry(p.instanceIndex, p.paramIndex) }))
};
oscQueryBridge.sendPacket(writePacket(message));
} catch (err) {
dispatch(showNotification({
level: NotificationLevel.error,
title: `Error while trying to update parameter list of SetView "${setView.name}"`,
message: "Please check the console for further details."
}));
console.log(err);
}
};

export const increaseParameterIndexInSetView = (setView: GraphSetViewRecord, param: ParameterRecord): AppThunk =>
(dispatch) => {
try {
const currentIndex = setView.params.findIndex(entry => entry.instanceIndex === param.instanceIndex && entry.paramIndex === param.index);
if (currentIndex >= setView.params.size) return;

const newList = setView.params
.delete(currentIndex)
.insert(currentIndex + 1, { instanceIndex: param.instanceIndex, paramIndex: param.index });

const message = {
address: `/rnbo/inst/control/sets/views/list/${setView.id}/params`,
args: newList.toArray().map(p => ({ type: "s", value: instanceAndParamIndicesToSetViewEntry(p.instanceIndex, p.paramIndex) }))
};
oscQueryBridge.sendPacket(writePacket(message));
} catch (err) {
dispatch(showNotification({
level: NotificationLevel.error,
title: `Error while trying to update parameter list of SetView "${setView.name}"`,
message: "Please check the console for further details."
}));
console.log(err);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i probably would have just made this an offsetParameterIndexInSetView(.., offset: number)

Comment on lines +5 to +6
export const genericMemo: <P>(component: P) => P = memo;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused?

address: "/rnbo/inst/control/sets/views/create",
args: [
{ type: "s", value: name },
...params.valueSeq().map(p => ({ type: "s", value: p.setViewId })).toArray()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to add parameters sort of randomly, i think they should be sorted by instance_index then index just like you get when you do the Include All in the set view param editor

@fde31
Copy link
Member Author

fde31 commented Dec 20, 2024

MetaData editing isn't a problem yeah. If the runner can handle the fact that for a SetView we'd basically arm multiple instances at the same time for MIDI mapping via /midi/last/report then I might as well just add that right away?

@x37v
Copy link
Contributor

x37v commented Dec 20, 2024

MetaData editing isn't a problem yeah. If the runner can handle the fact that for a SetView we'd basically arm multiple instances at the same time for MIDI mapping via /midi/last/report then I might as well just add that right away?

I'm not 100% on that but.. might work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants