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

Skins / Qt6: menu icon position fix #12216

Merged
merged 2 commits into from
Jun 2, 2024

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Oct 25, 2023

... + some cleanup

Menu icons are are a bit off to the left.
Screenshot_2024-05-29_02-19-51


With Qt6 the context menus should look like this:
(yes, this is expected to break styles with Qt5)
image
image
image
image
(yes, the shortcut Ctrl+a still overlaps "Select all")
image

Please verify on all OS and in all skins. Thanks!

  • main menu (menubar)
  • table header menu
  • track menu (decks & library), especially Crates menu
  • sidebar menu, especially Crates menu
  • edit fields menu (searchbar, track table inline editing, beatsize spinboxes etc.)

@github-actions github-actions bot added the skins label Oct 25, 2023
@ronso0 ronso0 added the QT6 label Oct 25, 2023
@JoergAtGithub
Copy link
Member

JoergAtGithub commented Oct 25, 2023

This is how it looks on Windows 11 with this PR:
grafik
grafik
grafik
I wasn't able to edit the track title in the library (BTW, should there be a tooltip, which explains in-place editing?)
grafik
Why is this menu in English language and all other in German?

@ronso0
Copy link
Member Author

ronso0 commented Oct 25, 2023

Thanks @JoergAtGithub

You only need to post screenshots if you notice significant deviation from my screenshots.

Why is this menu in English language and all other in German?

Good question. On Linux, the selected locale is applied to all menus.
Not a regression of this PR, I just touched the stylesheets. Please file an Issue.

1 similar comment
@ronso0
Copy link
Member Author

ronso0 commented Oct 25, 2023

Thanks @JoergAtGithub

You only need to post screenshots if you notice significant deviation from my screenshots.

Why is this menu in English language and all other in German?

Good question. On Linux, the selected locale is applied to all menus.
Not a regression of this PR, I just touched the stylesheets. Please file an Issue.

@ronso0
Copy link
Member Author

ronso0 commented Oct 29, 2023

BTW, should there be a tooltip, which explains in-place editing?

Maybe, though I have no idea where that would go. Having it appended to every cell's tooltip seems a bit annoying, especially if you're already aware of the feature.
I think this is part of the meta issue: how to properly introduce all those fancy UI features, i.e. increase discoverability of features like track drag'n'drop, double-click on track labels, skin settings and what not.

Copy link

github-actions bot commented Feb 6, 2024

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Feb 6, 2024
@ronso0 ronso0 added this to the 2.5-beta milestone May 29, 2024
@JoergAtGithub
Copy link
Member

Please fix the conflicts

@ronso0 ronso0 changed the base branch from main to 2.5 May 29, 2024 12:06
@ronso0
Copy link
Member Author

ronso0 commented May 29, 2024

I rebased onto 2.5.
Please check again on all targets:

  • table header menu
  • any QLineEdit menu (search, beat spinboxes, table inline editors, etc.)
  • track menu -> crates
  • sidebar -> crate name menu (with checkbox)
  • "search related tracks" menu
  • ...

@ronso0 ronso0 marked this pull request as ready for review May 29, 2024 20:49
@JoergAtGithub
Copy link
Member

Overall it looks good on Windows11 (Qt6.5.3 from VCPKG). But the checked checkboxes look slightly smaller than the unchecked and have rounder corners.

100% Zoom:
grafik

200% Zoom:
grafik

@daschuer
Copy link
Member

I can confirm the issue is fixed:
grafik

I cannot confirm the different checkbox sizes. @JoergAtGithub is this a regression form this PR?

@daschuer daschuer modified the milestones: 2.5-beta, 2.5.0 Jun 2, 2024
@JoergAtGithub
Copy link
Member

The differnent checkbox size is not a regression by this PR!

@ronso0
Copy link
Member Author

ronso0 commented Jun 2, 2024

Okay, so we need a check on macOS @m0dB @foss- @fwcd (?)

I'll take a look at the checkbox issue on Windows but will probably open a separate PR for this.

@foss-
Copy link
Contributor

foss- commented Jun 2, 2024

Crate checkbox + contextual menu for crate. There are no icons in the menu-bar options on macOS.
Version tested: 2.5-alpha-410-g7d4dcb92b7 (HEAD)

crate checkbox
contextual menu for crate
menu

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

Thank you. This means macOS is not affected and we can merge.

@daschuer daschuer merged commit 2c66ea3 into mixxxdj:2.5 Jun 2, 2024
13 checks passed
@ronso0 ronso0 deleted the menu-icon-position-fix branch June 2, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QT6 skins stale Stale issues that haven't been updated for a long time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants