D27481: Panel spacer that can center things

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:7b3b06a48576: Panel spacer that can center things 
(authored by mart).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27481?vs=76369&id=76372

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/panelspacer/CMakeLists.txt
  applets/panelspacer/Messages.sh
  applets/panelspacer/contents/config/main.xml
  applets/panelspacer/contents/ui/main.qml
  applets/panelspacer/metadata.desktop
  applets/panelspacer/package/contents/config/main.xml
  applets/panelspacer/package/contents/ui/main.qml
  applets/panelspacer/package/metadata.desktop
  applets/panelspacer/plugin/CMakeLists.txt
  applets/panelspacer/plugin/panelspacer.cpp
  applets/panelspacer/plugin/panelspacer.h
  shell/panelconfigview.cpp

To: mart, #plasma, #vdg, davidedmundson
Cc: davidedmundson, davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Marco Martin]

mart marked 7 inline comments as done.
mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in panelconfigview.cpp:115
> Seems like Panel could benefit from some sort of...ContinamentLayoutManager?
> 
> Effectively we're evaluating what RowLayout evaluates by hand here anyway, it 
> seems like the right path forward and worth investing time in. Though it can 
> happen after this patch.

indeed.

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidedmundson, davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Marco Martin]

mart updated this revision to Diff 76369.
mart added a comment.


  - take maximum size into account
  - document -1

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27481?vs=75990&id=76369

BRANCH
  mart/panelSpacer

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/panelspacer/CMakeLists.txt
  applets/panelspacer/Messages.sh
  applets/panelspacer/contents/config/main.xml
  applets/panelspacer/contents/ui/main.qml
  applets/panelspacer/metadata.desktop
  applets/panelspacer/package/contents/config/main.xml
  applets/panelspacer/package/contents/ui/main.qml
  applets/panelspacer/package/metadata.desktop
  applets/panelspacer/plugin/CMakeLists.txt
  applets/panelspacer/plugin/panelspacer.cpp
  applets/panelspacer/plugin/panelspacer.h
  shell/panelconfigview.cpp

To: mart, #plasma, #vdg
Cc: davidedmundson, davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27481: Panel spacer that can center things

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> broulik wrote in main.xml:15
> Please mention in the label what `-1` means

this needs doing

> main.qml:109
> +if (root.horizontal) {
> +middleItemsHint += Math.max(child.Layout.minimumWidth, 
> child.Layout.preferredWidth) + panelLayout.rowSpacing;
> +} else {

If we're trying to recreate the size hint of RowLayout we need to include 
maximum size hints

> mart wrote in panelconfigview.cpp:115
> so, i need to add the second spacer at first position rather than the last.
> I don't think i can do this without a script, as i need to access  the 
> qquickitem to actually set a position (well, i could by accessing the 
> graphics object property,directly, like the script implementation is doing, 
> but it would be uglier

Seems like Panel could benefit from some sort of...ContinamentLayoutManager?

Effectively we're evaluating what RowLayout evaluates by hand here anyway, it 
seems like the right path forward and worth investing time in. Though it can 
happen after this patch.

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidedmundson, davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Nathaniel Graham]

ngraham added a comment.


  That looks nice!

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidedmundson, davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Marco Martin]

mart added a comment.


  In D27481#614250 , @davidre wrote:
  
  > In D27481#614167 , @davidre 
wrote:
  >
  > > I think the deisgn og the spacer could be better in edit mode to clearly 
mark it as a spacer but that can be done in the future if desired.
  >
  >
  > An idea could be to display something similiar to the icon on the "Add 
spacer" button on then or an arrow on each side
  
  
  yeah, something along the lines (either one of those different two)
  F8114744: image.png 

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidedmundson, davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> davidedmundson wrote in panelconfigview.cpp:115
> why is this a script?

so, i need to add the second spacer at first position rather than the last.
I don't think i can do this without a script, as i need to access  the 
qquickitem to actually set a position (well, i could by accessing the graphics 
object property,directly, like the script implementation is doing, but it would 
be uglier

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidedmundson, davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> panelconfigview.cpp:115
> +// this to trigger the panel centering mode of the spacer in a slightly 
> more discoverable way
> +c->evaluateScript(QStringLiteral("panel = panelById(") + 
> QString::number(m_containment->id()) 
> ++ QStringLiteral(");"

why is this a script?

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidedmundson, davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Marco Martin]

mart added a comment.


  In D27481#614250 , @davidre wrote:
  
  > In D27481#614167 , @davidre 
wrote:
  >
  > > I think the deisgn og the spacer could be better in edit mode to clearly 
mark it as a spacer but that can be done in the future if desired.
  >
  >
  > An idea could be to display something similiar to the icon on the "Add 
spacer" button on then or an arrow on each side
  
  
  yeah, i was planning on something quite simple, like H looking
  anyways, the spacer should probably become a Loader and have all the edit 
mode graphics loaded only then (otherwise, empty loader)

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Marco Martin]

mart added a comment.


  In D27481#614250 , @davidre wrote:
  
  > In D27481#614167 , @davidre 
wrote:
  >
  > > I think the deisgn og the spacer could be better in edit mode to clearly 
mark it as a spacer but that can be done in the future if desired.
  >
  >
  > An idea could be to display something similiar to the icon on the "Add 
spacer" button on then or an arrow on each side
  
  
  any further change in design of the edit more graphics should be in a 
separate patch, it's besides the point here

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[David Redondo]

davidre added a comment.


  In D27481#614167 , @davidre wrote:
  
  > I think the deisgn og the spacer could be better in edit mode to clearly 
mark it as a spacer but that can be done in the future if desired.
  
  
  An idea could be to display something similiar to the icon on the "Add 
spacer" button on then or an arrow on each side

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Nathaniel Graham]

ngraham added a comment.


  Yeah it would be nice to call this an "Expanding spacer" or something like 
that. That's the term that we use for the rough equivalent for KXMLGui toolbars.

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[David Redondo]

davidre added a comment.


  I think the deisgn og the spacer could be better in edit mode to clearly mark 
it as a spacer but that can be done in the future if desired.

REPOSITORY
  R120 Plasma Workspace

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

To: mart, #plasma, #vdg
Cc: davidre, broulik, ngraham, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D27481: Panel spacer that can center things

[Marco Martin]

mart updated this revision to Diff 75990.
mart added a comment.


  - bigger in edit mode

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27481?vs=75988&id=75990

BRANCH
  mart/panelSpacer

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/panelspacer/CMakeLists.txt
  applets/panelspacer/Messages.sh
  applets/panelspacer/contents/config/main.xml
  applets/panelspacer/contents/ui/main.qml
  applets/panelspacer/metadata.desktop
  applets/panelspacer/package/contents/config/main.xml
  applets/panelspacer/package/contents/ui/main.qml
  applets/panelspacer/package/metadata.desktop
  applets/panelspacer/plugin/CMakeLists.txt
  applets/panelspacer/plugin/panelspacer.cpp
  applets/panelspacer/plugin/panelspacer.h
  shell/panelconfigview.cpp

To: mart, #plasma, #vdg
Cc: broulik, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart