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

Fix track color background with Qt6 #12380

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Nov 30, 2023

Fixes #12355 for me, both with Qt5 and Qt6 (note I didn't see #11630).

So, the issue is that 1ef4e4d

/* Prevent OS-style checkbox from being rendered underneath the custom style. */
WTrackTableView::item,
#LibraryBPMButton::item {
  border: 0px;
}

clears the track color set via BackgroundRole.

Seems it sufficient to call TableItemDelegate::paintItemBackground like it's done for the other delegates.

I don't know how to apply this to the Played checkbox (it's a native QTableView::indicator), so I added a delegate for that, too. Dumb c/p from BPM delegate, without the editor.

Please check if that works also on Windows and macOS @fwcd @JoergAtGithub
Should look good with all skins.

@fwcd
Copy link
Member

fwcd commented Nov 30, 2023

Looks good to me

@ronso0
Copy link
Member Author

ronso0 commented Dec 1, 2023

Can someone tell if the last commits makes sense?
I just feeled weird that every delegate includes QTableView etc.

@daschuer
Copy link
Member

daschuer commented Dec 1, 2023

The last commit has some implications regarding compile time. It includes unnecessarily headers into all cpp files that are including "src/library/tableitemdelegate.h"
The tool https://github.com/include-what-you-use/include-what-you-use points that out suggesting to remove all headers that are not used in the including files itself.

@daschuer
Copy link
Member

daschuer commented Dec 1, 2023

I have used this: daschuer@7f2c8ed
to do the latest header clean up. Unfortunately it is a bit too aggressive, suggesting to use internal Qt headers.

@ronso0 ronso0 force-pushed the lib-checkmark-fix branch from 3e0b96f to 866cea5 Compare December 1, 2023 08:54
+ some small improvements for the table view Shade
@ronso0 ronso0 force-pushed the lib-checkmark-fix branch from 866cea5 to b56244b Compare December 1, 2023 10:38
@ronso0
Copy link
Member Author

ronso0 commented Dec 1, 2023

Okay, thanks. I reverted that header commit.

Ready to roll if it's been confirmed to fix the issue on Windows, too.

@JoergAtGithub
Copy link
Member

I tested on Win11. The track color, checkbbox and padlock symbol appear as expected on all skins!

But there seems to be a missing refresh/update, after the color unassign operation. The update happens delayed, when I select another row:
https://github.com/mixxxdj/mixxx/assets/64457745/d4a3d7de-78ed-4153-844f-c71a509a366c

@ronso0
Copy link
Member Author

ronso0 commented Dec 1, 2023

Thanks for testing.

The refresh issue occurs also in 2.4 IIRC, and it affects the entire row. This PR only touches the BPM delegate, and the new one, of course.

@JoergAtGithub
Copy link
Member

Ok, if it's no regression - than we can merge this! Thank you!

@JoergAtGithub JoergAtGithub merged commit 6acfe63 into mixxxdj:main Dec 1, 2023
9 of 10 checks passed
@ronso0 ronso0 deleted the lib-checkmark-fix branch December 1, 2023 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

regression in main: library track colors no longer appear
4 participants