Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Can you explain what this change is all about, and provide before-and-after screenshots in the Test Plan section?
I have reduced the breeze layout and it looks nicer and cleaner than the current style. I attach photos.
Before
After
Yeah I don't think we are going to just randomly change that many constants based on two screenshots. This needs a wide discussion imo with what goals to achieve and the problems with the current breeze and much more comparisons.
I know that we periodically get people complaining that Breeze widgets are too large, and this is would be one way to address those concerns. However how does this look with different fonts and font sizes? As @davidre said, this needs more discussion, though thank you for submitting a patch.
BTW this patch does not apply because the file changed has an extra breeze/ prefix that is not needed. Please remove that.
I have updated the patch to comply with the requirements. Unfortunately, the Breeze widgets are large and I have made it more streamlined. You are right about the fact that you should try other characters too.
I personally like the changes made, based on the screenshots, though TBH the buttons should probably be 1px higher or something... they seem a little too short to me.
The reason why I suggested testing with other fonts is because the current interior padding is based on the metrics of the letter "M" of the active font. It just so happens that our default font Noto Sans yields greater padding than many other sans-serif fonts used in user interfaces. This means that reducing the padding to make controls look smaller when using Noto Sans results in controls being much smaller when using other fonts with different metrics. For example, here's how your patch looks when I change Noto Sans out for Ubuntu:
Those buttons are much too cramped IMO. Also, there are visual glitches of arrows being too bit and everything being shifted over to the right--which are technical issues with this patch that are solvable. However even if those are solved, I'm not sure this makes sense. I feel that the perceived issue of widgets being too large is due to Noto's peculiar metrics and if we want to do this, we should attack the problem from another angle--such as using metrics from a different letter, setting a hard cap on the padding for fonts with larger font metrics, or rethinking whether we want to derive widget padding from font metrics in the first place.
Also checkbox and radio button controls do not appear to be adjusted, so they now look too large in relation to other controls.
Hi, I modified the patch by checking the current metric and it seems to have achieved the desired effects.
Noto Sans
Dejavu sans
Inconsolata font
Arial
Liberation mono
Checkbox and radiobutton resize size
I don't see any metric checking in the current patch. Does its current state not include all changes?
Please follow the existing coding style. Can you explain the need for all these new setFont() calls? In general we need more explanation for these kinds of proposed changed. Keep in mind that your reviewers are not inside your head. :)
Good afternoon, after some time I put back the patch available. What I did in essence is to make the Breeze theme buttons smaller, set for each widget the possibility of using any font by rewriting the font design via QPainter. It is not a clean solution, but apparently it works very well. I add screens with 1920x1080 resolution.
Example //Redesign of the font on the widget if(widget) painter->setFont(widget->font());
//Resolution 1920x1080