andrew-bibb / cmst

QT GUI for Connman
174 stars 37 forks source link

Wrong button layout in Technologies table widget #166

Closed tsujan closed 7 years ago

tsujan commented 7 years ago

Hello!

It's me again. The (minor) problem I wanted to report is about the On and Off buttons of the Technologies table widget. As you can see in the following screenshots, they aren't OK with Breeze (KDE's default) and Kvantum style engines:

1

The problem is that no width can be enforced on a tool button by adding spaces to its text while it's inside a cell. In the right screenshot, Breeze has put the icon in a bad place because the text is too wide. In the left screenshot, Kvantum has correctly seen the extra spaces after "Off" and since there wasn't enough space for the text, it has elided it. As for "On" button, its behavior is like Breeze.

The problem can be fixed easily by:

(1) Removing qpb02->setFixedSize(), qpb04->setFixedSize() and especially, the string "padding" from controlbox.cpp; and

(2) Adding these lines to the idButton ctor:

button->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Fixed);
...
...
layout->setAlignment(Qt::AlignCenter);

The result with Breeze and Kvantum:

2

I guess you wanted to center the button text with Fusion (the default Qt5 engine) but that's impossible because Fusion aligns the text of tool buttons to the left. Each style engine draws tool buttons in its own way. Both Breeze and Kvantum put the text at the center but Breeze puts the icon at the center too while Kvantum aligns it to the left and puts the text at the center of the remaining space. Also each style engine reacts to a too long text differently: Breeze puts the icon in a wrong place while Kvatum also tries to elide the text (in this case, invisible spaces).

andrew-bibb commented 7 years ago

You give me too much credit for wanting to center the button text. Back in the early days of the project I was getting a great deal of comments from someone with a very detailed eye. He was bothered by the fact that the text jumped slightly changing from "on" to "off" using some GTK theme long since lost to history. I finally tracked it down to the fact that "on" has 2 letters and "off" has 3. The padding hack made that go away.

Anyway I've made your changes and uploaded them here. If you want to test them out and let me know if it works that would be great. On my system (no theme of any sort) the text is left aligned next to the colored golf ball, and really does not bother me at all, but if it works on your system as intended I'll figure I did it right. If I get a few minutes (not likely this month) I may look to see if I can get the text centered using style sheets.

I'm going to be away for next 4-6 days so no rush in getting back to me with this.

tsujan commented 7 years ago

You give me too much credit for wanting to center the button text.

Sorry, while reading the code, I unwittingly did some wrong "mind reading" ;)

If you want to test them out ...

I'll test them tomorrow. Thanks!

On my system (no theme of any sort) the text is left aligned next to the colored golf ball...

Probably you use the default Qt style engine. That's how it draws tool buttons.

I may look to see if I can get the text centered using style sheets.

Unfortunately, style sheets can create problems easily. With one Qt theme engine, they may look OK but with another, they may create weird things because of their fixed values for margins, spacing, etc, I don't mean that they can't be used consistently but, sometimes, they aren't worth the effort.

tsujan commented 7 years ago

It works fine with all Qt5 style engines now. Thanks! Closing this issue.

andrew-bibb commented 7 years ago

Saw the comment about style sheets. Works for me as I I really don't have time to play with them anyway.

Thanks for closing this as well. It is a simple thing, but it saved me from having to do it, and right now I really appreciate that.