fix layout size hints for button labels
ClosedPublic

Authored by mart on Feb 17 2020, 2:46 PM.

Details

Summary

icon sizes and label placements tested correct in several scenarios

  • control.icon.width/height is used as maximum size of the icon
  • if the button is smaller icons always scale down
  • icons stay ccentered regardless of button size when there is no text
Test Plan

fixes icon sizes without workarounds like D27260

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Feb 17 2020, 2:46 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 17 2020, 2:46 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
mart requested review of this revision.Feb 17 2020, 2:46 PM
mart edited the test plan for this revision. (Show Details)Feb 17 2020, 2:52 PM
davidedmundson requested changes to this revision.Feb 18 2020, 11:52 AM
davidedmundson added a subscriber: davidedmundson.

Please update tests/components to have it failing before and now passing.

This revision now requires changes to proceed.Feb 18 2020, 11:52 AM
mart updated this revision to Diff 75922.Feb 18 2020, 3:09 PM
  • some quirks to make it more similar to pc2
mart added a comment.Feb 18 2020, 3:10 PM

latest version, plasmacontrols 2 and 3 one beside the other

davidedmundson added inline comments.Feb 18 2020, 3:56 PM
tests/components/button3.qml
90–125

this doesn't contain an icon, which is the majority of what this patch is about

mart updated this revision to Diff 75927.Feb 18 2020, 4:06 PM
  • add an icon
mart marked an inline comment as done.Feb 18 2020, 4:07 PM
mart added inline comments.
tests/components/button3.qml
90–125

added

davidedmundson added inline comments.Feb 18 2020, 5:06 PM
tests/components/button3.qml
90–125

You didn't even run this :(

  1. icon.name not icon.source
  2. now the comment next to it is wrong
  3. there's a binding loop
  1. if I make a test without any text, which is what the thing you're supposedly fixing was doing, I don't get an icon
mart updated this revision to Diff 75972.Feb 19 2020, 9:41 AM
mart marked an inline comment as done.
  • fix button display property
  • add a test for tabbar
mart marked an inline comment as done.Feb 19 2020, 9:41 AM
mart added inline comments.
tests/components/button3.qml
90–125

sorry,
tested, fixed the problem and added new tests

src/declarativeimports/plasmacomponents3/Button.qml
62

I don't understand this part about constraining the width to the parent height (for all 3)

If I remove it all the tests still pass.

mart updated this revision to Diff 75976.Feb 19 2020, 11:06 AM
mart marked an inline comment as done.
  • remove extra check for squareness
mart added inline comments.Feb 19 2020, 11:07 AM
src/declarativeimports/plasmacomponents3/Button.qml
62

was an extra constraint to have buttons always square, but i added some test and removed this , they seem to behave always well without it

davidedmundson accepted this revision.Feb 19 2020, 11:08 AM
This revision is now accepted and ready to land.Feb 19 2020, 11:08 AM
This revision was automatically updated to reflect the committed changes.
ngraham added a subscriber: ngraham.EditedFeb 19 2020, 3:39 PM

The System Tray's pin button now looks like this for me:

It's fixed with D27260.

This patch also does not fix the media controller buttons' icon sizes, so D27449 is still needed. :/