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

Tweak remove/hide track confirm message to match confirm exit message #4577

Merged
merged 3 commits into from
Dec 29, 2021

Conversation

ninomp
Copy link
Contributor

@ninomp ninomp commented Dec 21, 2021

This is seeking to fix https://bugs.launchpad.net/mixxx/+bug/1947992.

This is a follow-up of #4330.

@github-actions github-actions bot added the ui label Dec 21, 2021
@foss-
Copy link
Contributor

foss- commented Dec 25, 2021

Testing 2.4-alpha-1034-g91bcb96ec6 (HEAD) from https://github.com/mixxxdj/mixxx/actions/runs/1608937872 does not resolve the UI issue reported in https://bugs.launchpad.net/mixxx/+bug/1947992:
2021-12-25

@ronso0
Copy link
Member

ronso0 commented Dec 25, 2021

@foss- Is the button appearance of the (hide message) identical for all skins?
I'm asking because with this PR the only difference between the "Close Mixxx?" and the "Confirm Track Hide" message is the parent widget: "Close Mixxx?" child of mixxxmainwindow, "Confirm Track Hide" child of WTrackTableView. The latter may have a stylesheet applied, with snippets like WLibrary QPushButton ....

@foss-
Copy link
Contributor

foss- commented Dec 26, 2021

This is more divers than anticipated.

Deere (all caps):
Screenshot 2021-12-26 at 19 53 22

Latenight:
Screenshot 2021-12-26 at 19 52 54

Shade (tiny buttons):
Screenshot 2021-12-26 at 19 54 48

Tango (expected):
Screenshot 2021-12-26 at 19 55 40

@ronso0
Copy link
Member

ronso0 commented Dec 26, 2021

Okay, so this needs to be addressed by changing the stylesheets. I'll take a look.

This PR LGTM

@ronso0
Copy link
Member

ronso0 commented Dec 26, 2021

@ninomp Feel free to pick the stylesheet fix 49d10e5
(I'm too lazy to open a PR atm :)

The dialog now looks good (system style).

…wlibrary' into tweak-remove-track-confirm-message
@github-actions github-actions bot added the skins label Dec 28, 2021
@ronso0
Copy link
Member

ronso0 commented Dec 28, 2021

@foss- Can you confirm the dialogs have the default system style now?

@foss-
Copy link
Contributor

foss- commented Dec 29, 2021

Confirming this is solved, dialog looks good in all skins.

@ronso0
Copy link
Member

ronso0 commented Dec 29, 2021

Great, thank you @ninomp for fixing and @foss- for testing :)

@ronso0 ronso0 merged commit 3b8295c into mixxxdj:main Dec 29, 2021
@ninomp ninomp deleted the tweak-remove-track-confirm-message branch December 29, 2021 21:12
@ninomp
Copy link
Contributor Author

ninomp commented Dec 29, 2021

@foss- Thank you for testing.
@ronso0 Thank you for assistance and review.

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.

3 participants