BUG 411811
Details
Diff Detail
- Repository
- R31 Breeze
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 18987 Build 19005: arc lint + arc unit
+1 conceptually, this will be nice to finally have fixed.
But might the opposite make more sense? If we make buttons shorter, we're slightly reducing their click targets, but if we make line edits taller, all we're doing it reducing unused whitespace.
From my personal experience, I didn't have any difficulty to click buttons of new sizes. And it seems the same height as buttons in Phabricator.
If we do make flat toolbuttons buttons (autoRaise == true) and non-flat buttons the same size, we will need to change the default icon size for all buttons to 22px, or we will have a ton of UI regressions. Any non-flat button with a fixed icon size is going to have a regression. Anyone who was using non-flat buttons with 22px icons to get a bigger clickable area will need to switch to 32px icons. This also means that breeze-icons' problem with 32px icon style consistency will show itself more often.
Thanks for the remind!
By default, QToolButton with 22px icons look good:
When change to 32px, still okay:
Maybe this is because Dolphin has all icons in 32px. Do you know any application that miss 32px icons in toolbar? Thanks!
This issue is most likely to show up in 3rd party apps. Maybe we shouldn't let an issue like that dictate the design of Breeze widgets though since I expect either Breeze icons or the XDG icon specs will change one day to fix that issue. For now, I think we should stick to reducing the size of non-flat buttons or increasing the size of comboboxes and line edits. If we make non-flat button sizes match flat button sizes, we should do that in another patch.
I personally think the height of line edits and comboboxes should be increased to 32px (using default font and icon settings). This also makes it so we won't have to change how flat button sizes work if/when the non-flat buttons are made to match them.
In QML applications, buttons and line edits are already same height, the same height as QLineEdit in Qt Widgets applications. If we increase the height of QLineEdit, we have to change QML components' height, too. The amount of work will be bigger. If a view contains a lot of QLineEdit/QComboBox rows (like many KCM views), the content might overflow. So it is safer to make button height shorter.
If this patch makes the sizes match those in Kirigami/QML apps, I guess that seems fine, since nobody has noticed any problems with the sized there, right?
The remaining 1px difference between QPushButton and QLineEdit is caused by content. Depending on fonts, the content size are always different.
Even in QML applications, like Elisa, we can notice the difference if we use different fonts:
Button height: 32px, LineEdit height: 34px. (Using Noto Sans SC fonts)
When applying size.setHeight(34); to each sizing function, they have exact same size. But the size doesn't respect users' font customization.
The best approach is to calculate font size directly, instead of relying on content size.
Force height consistency
Now they should have exactly same height (QPushButton, QToolButton, QLineEdit, QComboBox, QSpinBox):
Things seem noticeably too short to me now, and QWidgets things are smaller than their QML versions. Observe:
Above, QWidgets. Below, QML:
This seems like too invasive a change. I think we should fix this bug, but not by resizing everything.
I checked all KCM modules, the bottom row of buttons are all in same height (like the smaller button in your screenshot). Have you tried reboot?
This is what happens when icon size is set to 32px or more (48px here):
size.setHeight(toolButtonOption->fontMetrics.height()); doesn't take icon size into account.
Also, still -1 to making flat buttons and non-flat buttons the same size at the same icon sizes in this patch. That change has potential consequences that can reach much farther than the button vs line edit height inconsistency that originally needed to be solved.