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

lxqt-config-monitor: Change dialog button from QToolButton to QPushButton #1047

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isf63
Copy link
Contributor

@isf63 isf63 commented Sep 13, 2024

Change the "Settings" dialog button to a QPushButton. This simplifies the Designer UI file and fixes text/icon alignment (for example on Fusion.)

Change the "Settings" dialog button to a QPushButton. This simplifies the Designer UI file and fixes text/icon alignment
(for example on Fusion.)
@isf63 isf63 changed the title Change dialog button from QToolButton to QPushButton lxqt-config-monitor: Change dialog button from QToolButton to QPushButton Sep 13, 2024
@isf63
Copy link
Contributor Author

isf63 commented Sep 15, 2024

liblxqt uses QDialogButtonBox which itself uses QPushButton.

Re:

and fixes text/icon alignment (for example on Fusion.)

QToolButton has different (and in my opinion, worse) styling than QPushButton.

buttons

@tsujan
Copy link
Member

tsujan commented Sep 15, 2024

fixes text/icon alignment

I'm afraid there's nothing to fix. It's like saying, "Don't use tabs because they look ugly with Fusion."

@isf63
Copy link
Contributor Author

isf63 commented Sep 15, 2024

QToolButton is styled differently in most Qt styles. Logically dialog buttons should be consistently QPushButton as that is what liblxqt uses.

EDIT: And in Kvantum as well.

@tsujan
Copy link
Member

tsujan commented Sep 15, 2024

IMO, in no way should a code avoid using a widget because of how it's drawn by this or that Qt style.

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