Resize all item style
Needs ReviewPublic

Authored by angeloscarna on Apr 27 2020, 11:48 AM.


Group Reviewers

Diff Detail

Lint Skipped
Unit Tests Skipped
angeloscarna requested review of this revision.Apr 27 2020, 11:48 AM
angeloscarna created this revision.
angeloscarna added a reviewer: Breeze.
angeloscarna planned changes to this revision.Apr 27 2020, 12:16 PM
angeloscarna set the repository for this revision to R31 Breeze.Apr 27 2020, 12:56 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 27 2020, 12:56 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
angeloscarna requested review of this revision.Apr 27 2020, 12:57 PM
angeloscarna removed R31 Breeze as the repository for this revision.
angeloscarna removed a project: Plasma.
angeloscarna removed a subscriber: plasma-devel.
ngraham added a subscriber: ngraham.EditedApr 27 2020, 2:15 PM

Can you explain what this change is all about, and provide before-and-after screenshots in the Test Plan section?

angeloscarna added a comment.EditedApr 27 2020, 3:01 PM

I have reduced the breeze layout and it looks nicer and cleaner than the current style. I attach photos.



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 can add more screenshots or videos if you want.

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.

This needs to say ./kstyle/breeze.h

angeloscarna updated this revision to Diff 81356.
angeloscarna updated this revision to Diff 81357.
ngraham requested changes to this revision.Apr 27 2020, 5:15 PM

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.

This revision now requires changes to proceed.Apr 27 2020, 5:15 PM

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


Liberation mono

Checkbox and radiobutton resize size

angeloscarna updated this revision to Diff 81422.

I don't see any metric checking in the current patch. Does its current state not include all changes?

ngraham requested changes to this revision.Apr 29 2020, 1:52 PM

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. :)

This revision now requires changes to proceed.Apr 29 2020, 1:52 PM
angeloscarna abandoned this revision.Apr 30 2020, 6:28 PM
angeloscarna reclaimed this revision.May 13 2020, 2:04 PM

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.

//Redesign of the font on the widget

//Resolution 1920x1080

This revision now requires changes to proceed.May 13 2020, 2:04 PM