make the sizing of the toolbuttons more coherent
ClosedPublic

Authored by camiloh on Aug 22 2019, 10:36 PM.

Details

Summary

this makes the toolbutton icons follow a standard size.
Maybe one could make it follow the icon group size props? like icon.width and icon.height if those props are set.

Before patch:

After patch:

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.
camiloh created this revision.Aug 22 2019, 10:36 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptAug 22 2019, 10:36 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
camiloh requested review of this revision.Aug 22 2019, 10:36 PM
apol added a subscriber: apol.Aug 23 2019, 9:26 AM
apol added inline comments.
src/declarativeimports/plasmacomponents3/ToolButton.qml
65

That's always the case, isn't it? if a parent isn't visible, an item will never be visible.

camiloh marked an inline comment as done.Aug 23 2019, 4:07 PM
camiloh added inline comments.
src/declarativeimports/plasmacomponents3/ToolButton.qml
65

yes. you're right.

camiloh updated this revision to Diff 64432.Aug 23 2019, 4:13 PM
camiloh marked an inline comment as done.

if parent is nto visible then children are not visible. fix

camiloh edited the summary of this revision. (Show Details)Aug 23 2019, 4:13 PM

Not that it's an inappropriate fix, but are you sure you want to be using PlasmaComponents ToolButtons in Index? Why not use regular old QQC2 ToolButtons?

camiloh added a comment.EditedAug 23 2019, 7:32 PM

Not that it's an inappropriate fix, but are you sure you want to be using PlasmaComponents ToolButtons in Index? Why not use regular old QQC2 ToolButtons?

My intention with these patches is to "fix" visual issues affecting apps on PlaMo platform, issues that come from the Plasma style.
Apps using regular QQC2 components are no as nicely styled with the Plasma style as they are with others styles intended for standalone applications. some of the issues are affecting QCC2.ToolButton, QCC2.Button, QCC2.ToolBar... etc.
Index is using regular old QQC2 ToolButtons.

From the screenshots, the current style, makes QCC2 ToolButton be huge, taking the size from the layout. The idea is to make the ToolButton size standard

ngraham accepted this revision.Aug 23 2019, 9:20 PM

I see, that's just a test.

LGTM!

This revision is now accepted and ready to land.Aug 23 2019, 9:20 PM
This revision was automatically updated to reflect the committed changes.

This breaks the sizing of larger buttons like in media controller:

mart added inline comments.Aug 28 2019, 8:40 AM
src/declarativeimports/plasmacomponents3/ToolButton.qml
65

This fixes the icons to a single size no matter what, and wether it fits or not, and is going to break badly any time a bigger or smaller buttons is needed, as shown by breaking media controller.

I have to revert this now, the icon should scale only to "standard" sizes, but still scale with the button size.
if you really need fixed icons no matter what, this size hint may follow icon.width/icon.height from the control iff set to a valid measure

mart reopened this revision.Aug 28 2019, 8:41 AM
This revision is now accepted and ready to land.Aug 28 2019, 8:41 AM
mart requested changes to this revision.Aug 28 2019, 8:42 AM
This revision now requires changes to proceed.Aug 28 2019, 8:42 AM
mart added a comment.Aug 28 2019, 8:57 AM

Not that it's an inappropriate fix, but are you sure you want to be using PlasmaComponents ToolButtons in Index? Why not use regular old QQC2 ToolButtons?

The phone is using the plasma style for its qqc2

mart added inline comments.Aug 28 2019, 9:00 AM
src/declarativeimports/plasmacomponents3/ToolButton.qml
58

this further item enclosing is not necessary, is an object instantation more for no functional gain

64

This was ok as it was, including fillwidth/fillheight (perhaps)
but with the line:
Layout.preferredWidth: control.icon.width > 0 ? control.icon.width : -1
same for preferredHeight

camiloh updated this revision to Diff 64832.Aug 28 2019, 12:30 PM

make usage of the icon group properties: height and width

camiloh updated this revision to Diff 64844.Aug 28 2019, 1:23 PM

make the icon fill height and width if the icon group height or width props are not set or are 0

mart accepted this revision.Aug 28 2019, 1:38 PM
This revision is now accepted and ready to land.Aug 28 2019, 1:38 PM
This revision was automatically updated to reflect the committed changes.