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

enhance: menu #106

Merged
merged 22 commits into from
Jan 16, 2025
Merged

enhance: menu #106

merged 22 commits into from
Jan 16, 2025

Conversation

daiyam
Copy link
Contributor

@daiyam daiyam commented Jan 13, 2025

This PR homogenizes the different menus:

  • add missing icons
  • add Origin country on album
  • uses the artist's name or song's title when possible
  • hides options when there is no artist's name
  • fix menu position in the album's details

For the use of the artist's name/song's title, I changed the menu's labels to make them more readable.
It's a bit repetitive but it's clearer what's it will do.
Not sure if it's good. Tell me what's you think.

@daiyam
Copy link
Contributor Author

daiyam commented Jan 14, 2025

I'm thinking of consolidating #109 and #110 into this one...

What do you think?

@daiyam daiyam changed the title enhance: menu [wip] enhance: menu Jan 15, 2025
@basharovV
Copy link
Owner

Generally I like the changes, I'm just concerned about screen space because this menu isn't drawn outside of the window. So the options should all be useful but kept to a minimum (until we have a truly native floating menu, with submenus, etc)

  • I think that having a Wikipedia option when we already have Wiki panel might seem a bit redundant.
  • My instinct is to keep the queue menu specific to the queue, so just Remove, Open in Finder, Info & Metadata, no youtube/wiki options since it gets a bit too big
  • There's a bug - click outside to hide the menu doesn't work after 1) right-click on album, then 2) right-click on queue item

@daiyam
Copy link
Contributor Author

daiyam commented Jan 15, 2025

I think that having a Wikipedia option when we already have Wiki panel might seem a bit redundant.

The Wikipedia option gives more results than with the Wiki panel. But yeah, it's a bit redundant.

@daiyam
Copy link
Contributor Author

daiyam commented Jan 16, 2025

  • I think that having a Wikipedia option when we already have Wiki panel might seem a bit redundant.

When there is no results, I've added a message indicating so and links to search with Wikipedia or a search engine.
A setting to select the search engine might be good.

  • My instinct is to keep the queue menu specific to the queue, so just Remove, Open in Finder, Info & Metadata, no youtube/wiki options since it gets a bit too big

I've regrouped them in a submenu

  • There's a bug - click outside to hide the menu doesn't work after 1) right-click on album, then 2) right-click on queue item

The right click wasn't registered to hide the menu.

@daiyam daiyam changed the title [wip] enhance: menu enhance: menu Jan 16, 2025
@daiyam
Copy link
Contributor Author

daiyam commented Jan 16, 2025

@basharovV you should use the squash and merge when merging.
It's avoid to have the full history of a PR in the main branch.
And the PR becomes a single commit, so in the main branch's history, it's easier to see what's the PR have changed.

@basharovV
Copy link
Owner

Thanks for another great PR. Good use of types 👍 I like the submenu, maybe it should appear on hover instead of on click though, like system menus. But that can be handled separately, I'll merge this with squash now

@basharovV basharovV merged commit e67c50a into basharovV:main Jan 16, 2025
3 of 4 checks passed
@daiyam daiyam deleted the enhance-menu branch January 16, 2025 21:01
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