Differential D27460

fix layout size hints for button labels
ClosedPublic
Actions

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

Details

Reviewers
davidedmundson
Group Reviewers
Plasma
Commits
R242:ba0b637de94a: fix layout size hints for button labels
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)
Branch
phab/buttonslayout
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22663
Build 22681: arc lint + arc unit
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

davidedmundson added inline comments.Feb 19 2020, 10:13 AM
src/declarativeimports/plasmacomponents3/Button.qml
62 ↗(On Diff #75972)

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 ↗(On Diff #75972)

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
Closed by commit R242:ba0b637de94a: fix layout size hints for button labels (authored by mart). · Explain WhyFeb 19 2020, 11:12 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.

ngraham added a comment.Feb 19 2020, 3:42 PM

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

Showing Only Differences

This revision modifies 2 more files that are hidden because they were not modified between selected diffs and they have no inline comments.

Revision Contents
Changeset List

Diff 75976

View Options

src/declarativeimports/plasmacomponents3/Button.qml

Loading...
View Options

src/declarativeimports/plasmacomponents3/TabButton.qml

Loading...
View Options

src/declarativeimports/plasmacomponents3/ToolButton.qml

Loading...
View Options

tests/components/button3.qml

Loading...
View Options

tests/components/toolbutton3.qml

Loading...