Group Widget + Color Picker = Widget Out Of Bounds
Needs ReviewPublic

Authored by McPain on Sep 7 2018, 2:44 PM.

Details

Reviewers
ngraham
Group Reviewers
Plasma
Summary

BUG: 398356

Former we calculated a button size to fit the whole widget vertically (if the layout is horizontal) and vice versa but what if we don't have enough space to fit the widget horizontally?

We have a 100x100 field. Okay, let height = 90 (margins, etc)
Then, first button 90x90, spacer 5x90 (still okay), and one more button 90x90 (oops, fail: the widget is 185x90)

Now we check the second dimension and shrink the buttons when necessary.
First button 45x45, spacer 5x45, second button 45x45. The widget if 95x45, and it fits now.

So, we have to subtract spacer width from parent width and divide it by 2, because we have two buttons and check if it's smaller than height. If so, we fit in the width.

Same thing when layout is vertical.

Diff Detail

Repository
R114 Plasma Addons
Lint
Lint Skipped
Unit
Unit Tests Skipped
McPain created this revision.Sep 7 2018, 2:44 PM
Restricted Application added a project: Plasma. · View Herald TranscriptSep 7 2018, 2:44 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
McPain requested review of this revision.Sep 7 2018, 2:44 PM
cfeck added a subscriber: cfeck.Sep 27 2018, 12:41 AM

Any Plasma developer who has kdeplasma-addons installed to verify this?

Commits really benefit from a bit more description. Reviewers don't have the same context you had in your head when you wrote it.

applets/colorpicker/package/contents/ui/main.qml
111

Please explain the rationale behind this.

Especially the spacer.width being divided by 2 in the second part of the statement.

McPain added inline comments.Sep 27 2018, 8:17 AM
applets/colorpicker/package/contents/ui/main.qml
111

Especially the spacer.width being divided by 2 in the second part of the statement.

Look at all the parentheses I wrote ;)
I divide by two a *subtraction* of spacer.width from width.

Former we calculated a button size to fit the whole widget vertically (if the layout is horizontal) and vice versa but what if we don't have enough space to fit the widget horizontally?

We have a 100x100 field. Okay, let height = 90 (margins, etc)
Then, first button 90x90, spacer 5x90 (still okay), and one more button 90x90 (oops, fail: the widget is 185x90)

Now we check the second dimension and shrink the buttons when necessary.
First button 45x45, spacer 5x45, second button 45x45. The widget if 95x45, and it fits now.

So, we have to subtract spacer width from parent width and divide it by 2, because we have two buttons and check if it's smaller than height. If so, we fit in the width.

Same thing when layout is vertical.

McPain updated this revision to Diff 42413.Sep 27 2018, 8:19 AM

(height * 2) + spacer.height -> (height - spacer.height) / 2

McPain added inline comments.Sep 27 2018, 8:20 AM
applets/colorpicker/package/contents/ui/main.qml
138

We need to make thickness independent from length.

mart added a subscriber: mart.Nov 23 2018, 2:33 PM
mart added inline comments.
applets/colorpicker/package/contents/ui/main.qml
111

can you put this in the commit description as well?

McPain added inline comments.Nov 28 2018, 1:29 PM
applets/colorpicker/package/contents/ui/main.qml
111

How to get commit access?

McPain edited the summary of this revision. (Show Details)Dec 5 2018, 1:05 PM
McPain marked 4 inline comments as done.