D11410: [RFC] Add option to make applet fixed width

[Radek Hušek]

Pitel updated this revision to Diff 66260.
Pitel added a comment.


  Reabse & polish UI

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11410?vs=31036=66260

BRANCH
  fixedSizeOverride2

REVISION DETAIL
  https://phabricator.kde.org/D11410

AFFECTED FILES
  containments/panel/contents/config/main.xml
  containments/panel/contents/ui/ConfigOverlay.qml
  containments/panel/contents/ui/main.qml

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Marco Martin]

mart added a comment.


  any updates on this?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11410

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Marco Martin]

mart added a comment.


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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11410

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Radek Hušek]

Pitel updated this revision to Diff 31036.
Pitel added a comment.


  Fix tooltip text.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11410?vs=30728=31036

BRANCH
  fixedSizeOverride

REVISION DETAIL
  https://phabricator.kde.org/D11410

AFFECTED FILES
  containments/panel/contents/config/main.xml
  containments/panel/contents/ui/ConfigOverlay.qml
  containments/panel/contents/ui/main.qml

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Radek Hušek]

Pitel added inline comments.

INLINE COMMENTS

> mart wrote in ConfigOverlay.qml:385
> 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)

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

> mart wrote in ConfigOverlay.qml:402
> is it necessary to use a temp copy?

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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11410

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Marco Martin]

mart added a comment.


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

INLINE COMMENTS

> ConfigOverlay.qml:385
> +iconSource: "distribute-horizontal-x"
> +visible: currentApplet.expandingApplet
> +tooltip: i18n("Make applet non-expanding size")

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)

> ConfigOverlay.qml:402
> +}
> +var tmp = plasmoid.configuration.fixedSizeOverride
> +var index = tmp.indexOf(currentApplet.applet.id)

is it necessary to use a temp copy?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11410

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Radek Hušek]

Pitel updated this revision to Diff 30728.
Pitel added a comment.


  - Remove QtQuick.Controls import (was needed only by Checkbox)

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11410?vs=30555=30728

BRANCH
  fixedSizeOverride

REVISION DETAIL
  https://phabricator.kde.org/D11410

AFFECTED FILES
  containments/panel/contents/config/main.xml
  containments/panel/contents/ui/ConfigOverlay.qml
  containments/panel/contents/ui/main.qml

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Radek Hušek]

Pitel updated this revision to Diff 30555.
Pitel added a comment.


  - 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.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11410?vs=29940=30555

BRANCH
  fixedSizeOverride

REVISION DETAIL
  https://phabricator.kde.org/D11410

AFFECTED FILES
  containments/panel/contents/config/main.xml
  containments/panel/contents/ui/ConfigOverlay.qml
  containments/panel/contents/ui/main.qml

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Marco Martin]

mart added a comment.


  In D11410#229494 , @Pitel wrote:
  
  > 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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11410

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Radek Hušek]

Pitel updated this revision to Diff 29940.
Pitel added a comment.


  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

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11410?vs=29745=29940

BRANCH
  fixedSizeOverride

REVISION DETAIL
  https://phabricator.kde.org/D11410

AFFECTED FILES
  containments/panel/contents/config/main.xml
  containments/panel/contents/ui/ConfigOverlay.qml
  containments/panel/contents/ui/main.qml

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Marco Martin]

mart added a comment.


  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.

INLINE COMMENTS

> main.xml:13
> +
> +  list of items for which we set fillWidth = false
> +

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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11410

To: Pitel, #plasma
Cc: mart, hein, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Radek Hušek]

Pitel added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D11410

To: Pitel
Cc: mart, hein, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D11410: [RFC] Add option to make applet fixed width

[Radek Hušek]

Pitel created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
Pitel requested review of this revision.

REVISION 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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fixedSizeOverride

REVISION DETAIL
  https://phabricator.kde.org/D11410

AFFECTED FILES
  containments/panel/contents/config/main.xml
  containments/panel/contents/ui/ConfigOverlay.qml
  containments/panel/contents/ui/main.qml

To: Pitel
Cc: mart, hein, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol