Skip to content

Comments

Keep osara preset list open#1323

Merged
ScottChesworth merged 9 commits intojcsteh:masterfrom
ScottChesworth:keepOsaraPresetListOpen
Oct 31, 2025
Merged

Keep osara preset list open#1323
ScottChesworth merged 9 commits intojcsteh:masterfrom
ScottChesworth:keepOsaraPresetListOpen

Conversation

@ScottChesworth
Copy link
Collaborator

No description provided.

@github-actions
Copy link
Contributor

@ScottChesworth
Copy link
Collaborator Author

I goofed case sensitivity in the last PR, got confused, probably deleted the wrong thing, was easier to start a new PR. Sorry! This one has an Apply button and some extra access keys. I've added a label to the list box as well, might help with identifying it so Control+Space could work, haven't looked into that yet though.

@jcsteh
Copy link
Owner

jcsteh commented Oct 12, 2025

I'd still like to get your thoughts on this comment I made on the other PR:

This worries me a bit because anyone who has used this a lot is going to press enter expecting the dialog to close, but now it won't. Aside from people who don't read change logs (which is their own problem to an extent), this is a muscle memory problem. I can live with it if you think the gain is worth the pain - you're likely to be the one who has to support the aggrieved users - but it's worth thinking about.
An alternative would be to add an Apply button separate to the OK button with an access key of "a". Pressing OK (or enter) would apply and dismiss, but pressing alt+a would apply without dismissing.

You've added the Apply button now, but enter activates that and there's no OK button, so the muscle memory breakage still applies. Unless you don't think that's a problem in real usage?

readme.md Outdated
The Filter field allows you to filter the list to show only presets containing the entered text.
Pressing the OK button activates the preset that's currently selected in the list.
The Filter field (Alt+F) allows you to filter the list to show only presets containing the entered text.
Pressing Enter while focus is in the list box or Alt+A to click the Apply button loads the selected preset.
Copy link
Owner

Choose a reason for hiding this comment

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

Can we say "activate the Apply button"? I'm not a fan of using a mouse specific term like "click" for keyboard users. :) I'll grant this is pedantry, though. Same on the line below.

@ScottChesworth
Copy link
Collaborator Author

I'm pretty sure the gain is worth the pain. Before this, the filter feature was only useful for viewing preset names, now it's useful for hearing them in context too. Pretty big win with only minimal muscle memory disruption IMO.

@github-actions
Copy link
Contributor

@jcsteh
Copy link
Owner

jcsteh commented Oct 12, 2025

Thoughts on the alternative of having both Apply and OK? Enter to apply and dismiss, alt+a to just apply. That's a pretty common pattern.

@ScottChesworth
Copy link
Collaborator Author

I really like being able to have one hand on arrows and reach Enter from there, it's a wicked improvement when you're playing an instrument with one hand and testing filtered results with the other. Can we try it how it is now? I'll implement your suggestion if muscle memory breakage hits harder than I thought.

@ScottChesworth
Copy link
Collaborator Author

I can't figure out how to get play/pause working. Might have to go without that nicety unless you've got ideas (ideally ideas that are simple enough for me to try lol)

Copy link
Owner

@jcsteh jcsteh left a comment

Choose a reason for hiding this comment

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

Fair enough.

Supporting control+space is going to be annoying and I don't have the headspace to faf with it right now. I don't think we need to block on that though.

readme.md Outdated
To cancel a note preview, press the Control key.

### Navigating FX Presets Without Activating Them (Windows Only)
### Searching and navigating FX Presets Without Activating Them (Windows Only)
Copy link
Owner

Choose a reason for hiding this comment

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

nit: Navigating should have a capital n, since we're using title case in the rest of this heading.

ID_FX_PRESET_DLG DIALOGEX 250, 125, 200, 350
CAPTION "FX Preset"
BEGIN
LTEXT "&Presets:", IDC_STATIC, 10, 0, 40, 10
Copy link
Owner

Choose a reason for hiding this comment

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

Note that even if we support control+space eventually, we wouldn't use this label because it's localised. I don't mind whether we keep this or not, but I thought I'd flag this in case it's the only reason you added the label here.

@jcsteh
Copy link
Owner

jcsteh commented Oct 13, 2025

It's probably worth filing an issue to support control+space here so we don't forget.

@github-actions
Copy link
Contributor

@ScottChesworth ScottChesworth merged commit 36bec73 into jcsteh:master Oct 31, 2025
3 checks passed
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