[RFC] Add option to make applet fixed width
Needs ReviewPublic

Authored by Pitel on Mar 17 2018, 10:33 AM.

Details

Reviewers
None
Group Reviewers
Plasma
Summary

@hein refused D10944 with wish to move the config option to contaiment level.
This patch does it: It adds a IntList config option fixedSizeOverride
to panel containment, checkbox in config overlay to modify it, and
sets fillWidth and fillHeight of an applet to false if it is
in fixedSizeOverride array.

Note that the checkbox introduced by this patch still needs some polishing.

Diff Detail

Repository
R119 Plasma Desktop
Branch
fixedSizeOverride
Lint
No Linters Available
Unit
No Unit Test Coverage
Pitel created this revision.Mar 17 2018, 10:33 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 17 2018, 10:33 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
Pitel requested review of this revision.Mar 17 2018, 10:33 AM

Also do we want this or two options instead:

  • no expanding: maximumWidth = preferredWidth, and
  • no shrinking: minimumWidth = preferredWidth?

Because when attempting to use fillWidth = false with full version of task manager it will probably overflow the panel.

mart added a comment.Mar 19 2018, 2:39 PM

it's moving on the right track.. i would like the checkbox to appear like the one in the panel spacer, in the context menu could be tricky as would need to inject into the applet's contextualactons, so let's forget about it for now.

one thing that could be done is to put that button in the applet handle that appears on hover when the panel controller is open, with the same icon of the add panel spacer in a checkable toolbutton (just icon, tooltip on hover) the panel spacer should use the same config ui.

i would like to expose this ui only for applets that want to expand, and not for those that are already compact (i don't think this option would make any sense in kicoff for instance)

now, on the configuration side.. it's interesting the approach you took with a list of ids in the panel config, i tought more on something directly down in libplsma's applet, but this stays more confined to the panel, so i like it.

containments/panel/contents/config/main.xml
13

list of applet ids that should not have an expanding size policy to save space

Pitel updated this revision to Diff 29940.Mar 19 2018, 7:04 PM
In D11410#229277, @mart wrote:

it's moving on the right track.. i would like the checkbox to appear like the one in the panel spacer, in the context menu could be tricky as would need to inject into the applet's contextualactons, so let's forget about it for now.

I'm not sure that is good idea: There are already some items injected in context menus (e.g. Unlock widgets action) and looking at my panel some applets' context menus are not injected. I believe it is because they have multiple menus bound to specific parts of itself and only applet's global menu is (and can) be injected. Most notable examples are systray except the expansion arrow and task icon manager except empty space (if it has set fixedWidth I was unable to find a place in it which would show me context menu containing Unlock widget action).

one thing that could be done is to put that button in the applet handle that appears on hover when the panel controller is open, with the same icon of the add panel spacer in a checkable toolbutton (just icon, tooltip on hover) the panel spacer should use the same config ui.

The combox is currently in the popup showing on hover when panel controller is open. I can make it replace spacer's specific option. I can just remove spacer's option from its context menu and add some temporary code to convert spacer's option into the new one. (And a few version later we can drop this code.) Is it what you meant or did I misunderstand you?

i would like to expose this ui only for applets that want to expand, and not for those that are already compact (i don't think this option would make any sense in kicoff for instance)

Done.

Changes:

  • Fix config item label
  • Show checkbox only for expanding applets
  • A bit polish checkbox placement
Pitel marked an inline comment as done.Mar 19 2018, 7:05 PM
mart added a comment.Mar 23 2018, 1:11 PM
In D11410#229277, @mart wrote:

it's moving on the right track.. i would like the checkbox to appear like the one in the panel spacer, in the context menu could be tricky as would need to inject into the applet's contextualactons, so let's forget about it for now.

I'm not sure that is good idea: There are already some items injected in context menus (e.g. Unlock widgets action) and looking at my panel some applets' context menus are not injected. I believe it is because they have multiple menus bound to specific parts of itself and only applet's global menu is (and can) be injected. Most notable examples are systray except the expansion arrow and task icon manager except empty space (if it has set fixedWidth I was unable to find a place in it which would show me context menu containing Unlock widget action).

I'm fine with dropping the context menu action, would be nice for the spacer to share the same mechanism and ui, definitely!

one thing that could be done is to put that button in the applet handle that appears on hover when the panel controller is open, with the same icon of the add panel spacer in a checkable toolbutton (just icon, tooltip on hover) the panel spacer should use the same config ui.

The combox is currently in the popup showing on hover when panel controller is open. I can make it replace spacer's specific option. I can just remove spacer's option from its context menu and add some temporary code to convert spacer's option into the new one. (And a few version later we can drop this code.) Is it what you meant or did I misunderstand you?

i would like it as a checkable toolbutton with no label (but a tooltip) in handleRow, between configureButton and the label

Pitel updated this revision to Diff 30555.Mar 25 2018, 8:09 PM
  • Replace checkbox with a checkable button
  • Add support for panel spacer -- currently we just check whether the applet is panel spacer and if it is we use applet.configuration.expanding instead of our fixedSizeOverride. I think we should remove context action to set expanding from panel spacer after this is landed and then convert it to use fixedSizeOverride array.
Pitel updated this revision to Diff 30728.Mar 27 2018, 2:53 PM
  • Remove QtQuick.Controls import (was needed only by Checkbox)
mart added a comment.Mar 30 2018, 11:29 AM

almost there :)
I have a couple of points, then it can go :)

containments/panel/contents/ui/ConfigOverlay.qml
386

this will probably need as well
currentApplet.applet.pluginName == "org.kde.plasma.panelspacer"

or it gets lost when the spacer is configured as not expanding and i guess the spacer is special and we want this option to be always there
(step 2 would be removing the option in the spacer and make it use this system, but it's for a next commit)

403

is it necessary to use a temp copy?

Pitel added inline comments.Mar 30 2018, 7:33 PM
containments/panel/contents/ui/ConfigOverlay.qml
386

It does not, it is already in definition of expandingApplet (line 254 of main.qml).

403

My reasons for temp copy were:

  • push method has no effect on plasmoid.configuration.fixedSizeOverride (I assume this has something to do with the whole configuration magic behind but it might be worth further investigating.)
  • QML does not notify the change of property if the property is an array and you change only its elements (because the property still contains the same object) so we need to trigger this signal somehow.
Pitel updated this revision to Diff 31036.Mar 31 2018, 1:53 PM

Fix tooltip text.

mart added a comment.May 2 2018, 1:29 PM

we're here, mergein few days when 5.13 freezes and 5.14 branch opens

mart added a comment.Dec 3 2018, 8:58 AM

any updates on this?

Pitel updated this revision to Diff 66260.Sep 16 2019, 7:31 PM

Reabse & polish UI