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

Added option to play track from context menu dialog #790

Merged
merged 4 commits into from
Apr 22, 2022
Merged

Added option to play track from context menu dialog #790

merged 4 commits into from
Apr 22, 2022

Conversation

Bettehem
Copy link
Contributor

Hello again!
I have added an option to the context menu dialog which allows to play the track or add to queue. This solves #786.

There's one problem currently though. In the dialog where you can select to play now or add to queue, only arrow keys work.
When trying to use j and k, it scrolls the list in the background and not in the dialog, so this probably shouldn't be merged until that is solved.

@hrkfdn
Copy link
Owner

hrkfdn commented Apr 21, 2022

Hi there, thanks for the PR. Would love to merge this, but it appears to contain commit 9a97a6e from your other PR. Could you remove that commit?

@Bettehem
Copy link
Contributor Author

Oh, my bad, I hadn't noticed that. I have removed it now.

@hrkfdn hrkfdn merged commit b1f3984 into hrkfdn:main Apr 22, 2022
@hrkfdn
Copy link
Owner

hrkfdn commented Apr 22, 2022

Thank you! Merged :)

@Bettehem
Copy link
Contributor Author

Nice! There's a problem though, j and k don't work in the new dialog, only arrow keys do. j and k scrolls the list in the background of the dialog and not inside the dialog itself. Any suggestions how to fix this?

@hrkfdn
Copy link
Owner

hrkfdn commented Apr 25, 2022

@Bettehem: Ah, should be fixed with 52e2a53 :)

Two things were missing: the type in handle_move_command for the select items was not matching and commands were not forwarded to the context menu in the command handler. There's still a lot of boilerplate involved when creating context menus :(

@Bettehem
Copy link
Contributor Author

Oh I see! This is good to know if I work with context menus in the future. Thanks!

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