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

Update rest_slider.lua #696

Merged
merged 4 commits into from
Feb 15, 2024
Merged

Update rest_slider.lua #696

merged 4 commits into from
Feb 15, 2024

Conversation

cv-on-hub
Copy link
Contributor

Converting to modeless operation, in response to #691, required major surgery. Produces many undo_block entries on slider action but can find no way to avoid this while retaining continuous document screen updating.

Converting to `modeless` operation required major surgery. Produces many `undo_block` entries but can see no way to avoid this while keeping document screen updating.
@ThistleSifter
Copy link
Member

I haven't used the script, so I'm guessing based on what I've read here, but what about using RegisterHandleActivate? When the window is activated, start an undo block and when the windows is deactivated, end the block.

@cv-on-hub
Copy link
Contributor Author

what about using RegisterHandleActivate?

Great idea. I'd tried many other options but not the Activate handler. But unless I'm doing it wrong the Undo Stack is very picky and doing this fails to return the active tool to Selection at the end of the process. Same as every other failed option that doesn't track every single change that's interpreted as an Edit change.

Less important, but a single "wrapping" undo can't display the current (changing) score selection or layer number. I also wonder whether closing the dialog on OK would be better (though then resembling Modal operation) but I can't find the magic combination to achieve that with OkButtonCanClose.

Changed "OK" button to "Apply" to make more sense of modeless operation. Minor text edits.
@rpatters1
Copy link
Collaborator

rpatters1 commented Feb 14, 2024

You can make the OK button close by setting the OkButtonCanClose option inside the InitWindow handler. The reason it's a bit difficult is that the mixin library assumes you do not want to close on OK and resets it accordingly inside RunModeless.

I think the choice boils down to whether multiple undo entries is more annoying or modal operation is more annoying. Even if you set OkButtonCanClose you can't count on the OK button with a modeless. In fact the whole reason for the modeless is to be able to operate on multiple selection regions without closing the window. Speaking for myself, I prefer the modeless dialog, but others may feel differently.

There might be a way to notify when the slider button is released. I'll add a note to myself to investigate it. It wouldn't help if you were using arrow keys to move it, but there might be a way if you are using the mouse to move it. Either way, it will have to wait for another RGP Lua release.

@cv-on-hub
Copy link
Contributor Author

the mixin library assumes you do not want to close on OK

Ah yes - I remember this issue from ages ago (early foray into Modeless under mixin). Having initially agreed with your preference for Modeless in this case, now that it's up and running my preference has reverted to Modal, and that's the version I'll be using. It's cute to change the selection while the dialog is on-screen, but it only takes a handful of milliseconds to close a Modal, change selection and reopen the Modal (using KM for instance). I really hate the ambiguity of window focus in Modeless, especially after activating show_notes_dialog().

So this version can go on the repo if you like, along with the plethora of undo events. I use (of course) the key equivalents (+/- with fast key repeats) to change values so simply tracking the slider-thumb won't help. Have been discussing this offline with @ThistleSifter and probably run out of other workarounds for a less wasteful undo list.

There's been some good feedback on the original Modal version on FB (well ... one positive review!). Not sure it's a good idea to post both Modal and Modeless versions.

@rpatters1
Copy link
Collaborator

GIven the ambiguity, I think an option would preferable. It's literally only the difference between calling RunModeless and ExecuteModal (plus updating the undo handling.)

One of the things I dislike most about the modal version is that the selected region vanishes when the dialog opens.

FWIW: it looks like adding a mouse tracking started/stopped notification is going to be fairly easy. That will considerably reduce the number of undo entries with modeless.

@jwink75
Copy link
Collaborator

jwink75 commented Feb 14, 2024 via email

@cv-on-hub
Copy link
Contributor Author

cv-on-hub commented Feb 14, 2024

I think an option would preferable.

Are you suggesting posting both? I'd rename this one rest_slider_modeless. (In this case the conversion to modeless required jumping through multiple tricky hoops that I wouldn't replicate in a modal version). Or are you suggesting a "modeless/modal" checkbox? (for use on the next call).

@jwink75
Copy link
Collaborator

jwink75 commented Feb 14, 2024 via email

@rpatters1
Copy link
Collaborator

I'm suggestion one script with the option to be modal or modeless. The difference between the two (with mixins) is diminimous.

@rpatters1
Copy link
Collaborator

With mixins it is simpler that that, @jwink75:

if modeless_option then
	dialog:RunModeless()
else
	dialog:ExecuteModal()
end

There will also need to be code to suppress undo handling if it is modal.

@cv-on-hub
Copy link
Contributor Author

OK. A few more hoops to jump through converting back to modal (... all that timer stuff) but not too arduous. OPTIONAL modeless follows.

PS @jwink75: Page Format Wizard is magical and I have no problem changing Measurement Units - it was the fact that changing units in some areas changed the VALUES for data in a different area!

This creates optional modal/modeless run modes.
@rpatters1
Copy link
Collaborator

Looks excellent to me. When 0.72 comes out we can add mouse tracking events and reduce the number of modeless undo events. But that's gotta wait for 0.72.

FYI: there is currently no continuous update on Windows. Just the final value when the mouse is released. That will change with 0.72 so that both report continuously during mouse scrolling.

@rpatters1
Copy link
Collaborator

Let me know if you are ready, and I'll merge it. (Probably later tonight.)

a few tidying things (logic clarifies) and a few less conditionals.
@cv-on-hub
Copy link
Contributor Author

Let me know if you are ready, and I'll merge it.

Great. Just a few tidying things and a few less conditionals. All ready to go.

@rpatters1 rpatters1 merged commit 5e3a1ca into finale-lua:master Feb 15, 2024
@cv-on-hub cv-on-hub deleted the cv-rest-slider branch February 15, 2024 04:25
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.

4 participants