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

Use imported drumkit immediately on open from Finder #1918

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cme
Copy link
Contributor

@cme cme commented Dec 31, 2023

Previously, importing drumkit by opening it in Finder would import the kit, but not update the widget or provide any other feedback to let the user know it was actually successful, or allow them to start using it.

On reflection, a far more reasonable user expectation is that opening a drumkit in Finder should select that drumkit, even if it's already imported.

cme added 2 commits December 31, 2023 11:31
Previously, importing drumkit by opening it in Finder would import
the kit, but not update the widget or provide any other feedback
to let the user know it was actually successful, or allow them to
start using it.
@theGreatWhiteShark
Copy link
Contributor

Previously, importing drumkit by opening it in Finder would import the kit, but not update the widget or provide any other feedback to let the user know it was actually successful, or allow them to start using it.

It does not? Any error appearing in the logs?

On master I get both a dialog "drumkit was successfully imported" (or something similar) and the Sound Library widget is updated with the imported kit appearing as well.

Since #1905 (on master as well) this can be done by right-clicking within the Sound Library widget or using the "Drumkit" item in the main menu bar. But only for the latter the imported drumkit will be loaded right away (in order to emphasis that actions within the Sound Library do not affect the current song and those accessible from the menu do not affect the kits in the SL).

On reflection, a far more reasonable user expectation is that opening a drumkit in Finder should select that drumkit, even if it's already imported.

Right. What happens if the kit is already imported? I think right now we just overwrite the existing one which is not good. On the other hand, maybe the kit in the SL is a modified version and the user wants to reset it by reimporting it. Maybe we should show a dialog "Kit is already present" with the options load, replace, and cancel or similar.

@cme
Copy link
Contributor Author

cme commented Jan 1, 2024

The FileOpen event is macOS specific and is sent to the QApplication when the user opens something in Finder.

I think you're right, a confirmation dialog would be good. I should probably make the behaviour consistent between opening in Finder and using the Import dialog.

@theGreatWhiteShark
Copy link
Contributor

The FileOpen event is macOS specific and is sent to the QApplication when the user opens something in Finder.

Ah. I see. I thought you used the import option from within Hydrogen and Finder was used instead of a QFileDialog.

I should probably make the behaviour consistent between opening in Finder and using the Import dialog.

Yeah. Maybe we can put all of it into CoreActionController (and cover potentially missing GUI-related updates with a dedicated EventType). We also have an OSC event for importing kits (/Hydrogen/EXTRACT_DRUMKIT/ called with just one argument) as well as a command line option in h2cli. They should all behave the same.

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