D27608: Fixes for applet configuration layout.

2020-03-05 Thread George Vogiatzis
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:d6731530bb43: Fixes for applet configuration layout. 
(authored by gvgeo).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=77046=77050

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-05 Thread George Vogiatzis
gvgeo updated this revision to Diff 77046.
gvgeo added a comment.


  Remove title alingment for kirigami change.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=77017=77046

BRANCH
  configscollcat (branched from master)

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-05 Thread Filip Fila
filipf accepted this revision.
filipf added a comment.


  Good stuff

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-05 Thread George Vogiatzis
gvgeo updated this revision to Diff 77017.
gvgeo added a comment.


  Prevent vertical scrollbar from popping on/off internally.
  Fix title alignment for RTL.
  
  I tried splitting into two patches, to separate the height loop changes, but 
could not find a good point to cut the patch.
  All the loops messages are caused from the pageStack height. The loop already 
exist, but is more easily detected with the `ScrollBar.vertical.policy`.
  The big spacing in the video is a FormLayout issue, among many other things. 
Now is obvious that width does not always refresh. For example, when expanding 
window vertical only, the width changing from the scrollbar(that disappears) 
does not update.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=76910=77017

BRANCH
  configscollcat (branched from master)

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-04 Thread George Vogiatzis
gvgeo added a comment.


  There is a problem, I still try to fix...

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-04 Thread Nathaniel Graham
ngraham added a comment.


  Yeah, this seems fine to land as-is. :)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-04 Thread Filip Fila
filipf added a comment.


  In D27608#621705 , @gvgeo wrote:
  
  > Strange, I don't get the resizing problem as it is now, or in master. Only 
managed to get this space by changing the theme, and was immediately fixed with 
resize.
  >
  > Thanks Filip for double checking.
  
  
  No problem. It needs to be a specific kind of resize action I guess.
  
  F8149051: RTLresize 
  
  I'm using Kvantum but I can make it with happen Breeze as well. We probably 
also need to change heading so it's aligned to the right, but both of these 
things are material for a different patch.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-04 Thread George Vogiatzis
gvgeo added a comment.


  Strange, I don't get the resizing problem as it is now, or in master. Only 
managed to get this space by changing the theme, and was immediately fixed with 
resize.
  
  Thanks Filip for double checking.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-04 Thread George Vogiatzis
gvgeo updated this revision to Diff 76910.
gvgeo added a comment.


  Cancel button was touching the edge in RTL.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=76908=76910

BRANCH
  configscollcat (branched from master)

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-04 Thread Filip Fila
filipf accepted this revision.
filipf added a comment.


  Fixes the regression. It's still possible to get this when resizing, but it 
was actually already like that before this patch.
  
  F8149023: image.png 

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-04 Thread George Vogiatzis
gvgeo updated this revision to Diff 76908.
gvgeo marked an inline comment as not done.
gvgeo added a comment.


  Fixed margins.
  Unfortunatly the RTL layout has some issues, had to disable the use of 
available width for RTL layout, to stop the jumping from the scrollview.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=76869=76908

BRANCH
  configscollcat (branched from master)

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-03 Thread Filip Fila
filipf added a comment.


  Seems to run pretty well, but there is a tiny padding regression introduced.
  
  Using `plasmashell --reverse --replace` to test a right to left layout we see 
that there is excessive padding between the content and the sidebar.
  
  F8148094: image.png 
  
  I.e. there is more padding than when using a left to right layout, and when 
resizing the window the padding actually increases some more (to level that's 
shown in the screenshot)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-03 Thread George Vogiatzis
gvgeo marked an inline comment as done.
gvgeo added a comment.


  I'll give a bit more time, so @filipf gets a chance to check again and accept 
if possible.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-03 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Yep! Everything else looks good to me.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  configscollcat (branched from master)

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-03 Thread George Vogiatzis
gvgeo updated this revision to Diff 76869.
gvgeo edited the test plan for this revision.
gvgeo added a comment.


  Restore code for vertical line.
  Cannot find a reason this happens or replicate. But realized that the change 
was unnecessary.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=76356=76869

BRANCH
  configscollcat (branched from master)

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-03 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  So with this patch, I see a new visual issue: the first time I open an applet 
config window, the vertical separator is not visible: F8147412: 
Screenshot_20200303_093031.png 
  
  It becomes it becomes visible (and thereafter stays visible) if I resize the 
window or switch categories. I don't see this issue with the current state of 
git master, so it seems to be a regression.

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #plasma, #vdg, filipf, ngraham
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-03-03 Thread George Vogiatzis
gvgeo added a comment.


  > I'm starting to agree that this is the wrong place to add the titlebar 
separator.
  
  Can this go as is, for now? I don't believe there is an alternative in the 
near future.

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #plasma, #vdg, filipf
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-02-25 Thread Nathaniel Graham
ngraham added a comment.


  Visually +1.
  
  I'm starting to agree that this is the wrong place to add the titlebar 
separator.

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #plasma, #vdg, filipf
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-02-25 Thread George Vogiatzis
gvgeo added a comment.


  Can't have everything. now will cat line again.
  F8130965: Screenshot_20200225_140134.png 


REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #plasma, #vdg, filipf
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-02-25 Thread George Vogiatzis
gvgeo updated this revision to Diff 76356.
gvgeo added a comment.


  Categories touch top and bottom.
  Removed 1 unnecessary line.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=76320=76356

BRANCH
  configscollcat (branched from master)

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

To: gvgeo, #plasma, #vdg, filipf
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-02-25 Thread Filip Fila
filipf added a comment.


  Looks good to me and solves 3 of the 4 binding loops, although this one still 
remains:
  
  
`file:///usr/share/plasma/shells/org.kde.plasma.desktop/contents/configuration/AppletConfiguration.qml:296:25:
 QML StackView: Binding loop detected for property "height"`

INLINE COMMENTS

> gvgeo wrote in AppletConfiguration.qml:117
> I can make this side to draw on top of the separator, as it was. Will be 
> incorrect again.
> 
> Maybe separator must leave. It looks nice for breeze dark, but if the theme 
> says no line, why force it? It is not forced in system settings. And despite 
> if it looks okay or not for most setups, it is wrong to display for all 
> themes.

> Maybe separator must leave

I agree. I already tried doing it in D25728 
.

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #plasma, #vdg, filipf
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-02-24 Thread George Vogiatzis
gvgeo added inline comments.

INLINE COMMENTS

> AppletConfiguration.qml:117
>  Kirigami.Separator {
> -visible: configDialog.configModel
> -anchors.left: sidebar.right
> -height: root.height
> -}
> -
> -Kirigami.Separator {
> +id: topSeparator
>  anchors.top: root.top

I can make this side to draw on top of the separator, as it was. Will be 
incorrect again.

Maybe separator must leave. It looks nice for breeze dark, but if the theme 
says no line, why force it? It is not forced in system settings. And despite if 
it looks okay or not for most setups, it is wrong to display for all themes.

> AppletConfiguration.qml:152
> +Layout.preferredWidth: units.gridUnit * 7
> +Layout.bottomMargin: units.smallSpacing
> +contentWidth: -1

So that content does not touch the bottom edge, but was unsightly with double 
space..
I will remove, it. It looks so much better with the categories coming from the 
edge, and scrollbar having the same length with the rectangular.

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #plasma, #vdg, filipf
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-02-24 Thread Nathaniel Graham
ngraham added subscribers: filipf, ngraham.
ngraham added a reviewer: filipf.
ngraham added a comment.


  Looks quite sane overall, modulo a few questions and comments. Adding @filipf 
as a reviewer since I know he's done some work here too.

INLINE COMMENTS

> AppletConfiguration.qml:117
>  Kirigami.Separator {
> -visible: configDialog.configModel
> -anchors.left: sidebar.right
> -height: root.height
> -}
> -
> -Kirigami.Separator {
> +id: topSeparator
>  anchors.top: root.top

Hmm, mking the separator always span the full width introduces a row of pixels 
on top of the sidebar that is slightly unsightly  when the top category is 
focused: F8129488: Screenshot_20200224_143600.png 


> AppletConfiguration.qml:152
> +Layout.preferredWidth: units.gridUnit * 7
> +Layout.bottomMargin: units.smallSpacing
> +contentWidth: -1

What's this for?

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #plasma, #vdg, filipf
Cc: ngraham, filipf, 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


D27608: Fixes for applet configuration layout.

2020-02-24 Thread George Vogiatzis
gvgeo updated this revision to Diff 76320.
gvgeo added a comment.


  style restore

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=76319=76320

BRANCH
  configscollcat (branched from master)

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

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


D27608: Fixes for applet configuration layout.

2020-02-24 Thread George Vogiatzis
gvgeo updated this revision to Diff 76319.
gvgeo retitled this revision from "Fix applet configuration ovelap with top 
line" to "Fixes for applet configuration layout.".
gvgeo edited the summary of this revision.
gvgeo edited the test plan for this revision.
gvgeo added a comment.


  Added all the related planned changes.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27608?vs=76244=76319

BRANCH
  configscollcat (branched from master)

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

AFFECTED FILES
  desktoppackage/contents/configuration/AppletConfiguration.qml

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