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) apply macOS style to dynamically added sliders, too #12631

Closed
wants to merge 1 commit into from

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Jan 23, 2024

Closes #12630

@idcmp Please test with the CI build, see https://github.com/mixxxdj/mixxx/wiki/Testing for instructions.

@daschuer
Copy link
Member

@idcmp do you think you will find time to test this this weekend? Than it can be part of the 2.4.0 release.

Or can we just merge it without confirm?

@idcmp
Copy link

idcmp commented Jan 27, 2024

Hi! Sorry, I'm playing all weekend and too chicken to fiddle with things right now. The soonest I would feel safe to tackle this is Monday.

@fwcd
Copy link
Member

fwcd commented Jan 27, 2024

UI lgtm. Can confirm that the sliders feel "glitchy" though:

Screen.Recording.2024-01-27.at.23.09.29.mov

@JoergAtGithub
Copy link
Member

UI lgtm. Can confirm that the sliders feel "glitchy" though:

In your video they look broken

@fwcd
Copy link
Member

fwcd commented Jan 27, 2024

Yeah, maybe I phrased it wrong, I meant that the sliders look fine, but controlling them still doesn't feel quite right. Is this a macOS-only problem though? This feels like there's something wrong with the way the "view state" is synchronized with the "model" (I have no idea how the internals work here, so forgive me if I use the wrong terms here)

@fwcd
Copy link
Member

fwcd commented Jan 27, 2024

Ah, maybe I should've looked closer, there are some visual glitches there too..

@daschuer
Copy link
Member

daschuer commented Feb 1, 2024

How does controlling works main 2.3.6 and 2.3.4 beta? Let's go for a solution that behaves right, even it may looks broken.

@daschuer
Copy link
Member

daschuer commented Feb 1, 2024

On Ubuntu Focal 2.4, main and this branch look the same.

@ronso0
Copy link
Member Author

ronso0 commented Feb 1, 2024

Hi! Sorry, I'm playing all weekend and too chicken to fiddle with things right now. The soonest I would feel safe to tackle this is Monday.

@idcmp You can test PR builds without risk, just keep the installer of your current version and backup your settings/database: https://github.com/mixxxdj/mixxx/wiki/Testing

@fwcd
Copy link
Member

fwcd commented Feb 1, 2024

2.3 feels fine

Screen.Recording.2024-02-01.at.14.30.18.mov

Interestingly, the main branch with Qt 6 feels fine too:

Screen.Recording.2024-02-01.at.14.32.09.mov

Just 2.4 has some odd issues (perhaps because it still uses our hacked together Qt 5 build?):

Screen.Recording.2024-02-01.at.14.33.39.mov

@daschuer
Copy link
Member

daschuer commented Feb 1, 2024

I think the best solution would be to put the maximum number of expected slider to the ui file and just change the labels and hide the unused sliders.

@daschuer
Copy link
Member

daschuer commented Feb 2, 2024

@fwcd Can you check if this one is causing the flipping of the sliders:
#11663

@ronso0
Copy link
Member Author

ronso0 commented Feb 2, 2024

Oh, that one.
If that's the cause we can simply not apply that to the main EQ sliders. They're at the bottom of the page and the scroll-while-hovering aka catch unintended scroll events is much less likely there.

@daschuer
Copy link
Member

daschuer commented Feb 2, 2024

@fwcd
I have prepared #12711 with a static set of sliders.
Please check if they are also affected from the style and control glitch.
If not I will continue that route for a fix.

@daschuer
Copy link
Member

daschuer commented Feb 4, 2024

Since this does not we work, can we close it?

@daschuer daschuer removed this from the 2.4.0 milestone Feb 4, 2024
@fwcd
Copy link
Member

fwcd commented Feb 4, 2024

I think so and, if this is indeed a Qt bug as I suspect, we'd probably be better off patching it in the buildenv. See #12711 (comment)

@ronso0
Copy link
Member Author

ronso0 commented Feb 4, 2024

Yup, you can just backport the fix.
IIUC we can also revert #11647 then.

@ronso0 ronso0 closed this Feb 4, 2024
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.

Mixer > Main EQ > Graphic Equalizer isn't usable
5 participants