D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R108:cee5ea7819b2: [kcmkwin/desktop] KCM using new virtual 
desktops DBus interface (authored by hein).

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=47749&id=47750

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham, #documentation
Cc: yurchor, ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, 
mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 47749.
hein added a comment.


  - Add missing upper bound to the Rows spinbox (cf. 06a9a2a468df 
).
  - Change 'msec' to 'ms'.

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=47546&id=47749

BRANCH
  master

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham, #documentation
Cc: yurchor, ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, 
mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> davidedmundson wrote in kcm_kwin_virtualdesktops.desktop:6
> please make sure the docs people know about this

Told Luigi about it weeks ago, no worries :)

> yurchor wrote in main.qml:222
> Can this be just "ms"-symbol as in Wikipedia for milliseconds to avoid 
> confusion with plural forms? Thanks.

Sure, I just copied "msec" from the old KCM. Will adjust before pushing.

REPOSITORY
  R108 KWin

BRANCH
  master

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham, #documentation
Cc: yurchor, ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, 
mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Yuri Chornoivan]

yurchor added inline comments.

INLINE COMMENTS

> main.qml:222
> +
> +textFromValue: function(value, locale) { return i18n("%1 
> msec", value)}
> +

Can this be just "ms"-symbol as in Wikipedia for milliseconds to avoid 
confusion with plural forms? Thanks.

REPOSITORY
  R108 KWin

BRANCH
  master

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham, #documentation
Cc: yurchor, ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, 
mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, 
ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> kcm_kwin_virtualdesktops.desktop:6
>  X-KDE-ServiceTypes=KCModule
> -X-DocPath=kcontrol/desktop/index.html
> -Icon=preferences-desktop
> -Exec=kcmshell5 desktop
> +X-DocPath=kcontrol/kwin_virtualdesktops/index.html
>  

please make sure the docs people know about this

REPOSITORY
  R108 KWin

BRANCH
  master

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Vlad Zagorodniy]

zzag accepted this revision.
zzag added a comment.


  Please wait for David's review. :-)

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  In D14542#376755 , @hein wrote:
  
  > @ngraham From a UX perspective your comments make perfect sense, but 
unfortunately KWin currently has a flat list of desktops divided by number of 
rows so I need to decline that for now. It's out of scope for 5.15 for sure.
  
  
  Darn. Everything else looks and feels good to me. It would be nice to get the 
animation setting back in here for 5.15 though.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein added a comment.


  Apply button working for me: https://youtu.be/LvMhpCLxdWY

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein added a comment.


  In D14542#376846 , @zzag wrote:
  
  > Hmm, I'm no longer able to save new settings, i.e. if any option has been 
changed, the Apply button is still not enabled.
  
  
  Can you give me more specific steps? Because I can't reproduce here ...

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Vlad Zagorodniy]

zzag added a comment.


  In D14542#371677 , @zzag wrote:
  
  > - if any virtual desktop is removed, then System Settings window will be 
sent to the last virtual desktop.
  
  
  Should be addressed by D17576 .

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Vlad Zagorodniy]

zzag added a comment.


  Hmm, I'm no longer able to save new settings, i.e. if any option has been 
changed, the Apply button is still not enabled.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 47546.
hein added a comment.


  Fix initial state

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=47240&id=47546

BRANCH
  master

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein added a comment.


  @ngraham: From a UX perspective your comments make perfect sense, but 
unfortunately KWin currently has a flat list of desktops divided by number of 
rows.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, GB_2, mkulinski, 
ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Vlad Zagorodniy]

zzag added a comment.


  In D14542#375522 , @ngraham wrote:
  
  > Overall it works very well! UI-wise, I found myself confused by the 
unpredictability of the Add button. With multiple rows, it was not clear which 
row the new desktop would be added to. Perhaps instead we could give each row 
an inline Add button, kind of like this:
  
  
  I'm afraid that in order to do that, we have to change KWin core (and also 
some effects) first.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  I can confirm @zzag's bug.
  
  Overall it works very well! UI-wise, I found myself confused by the 
unpredictability of the Add button. With multiple rows, it was not clear which 
row the new desktop would be added to. Perhaps instead we could give each row 
an inline Add button, kind of like this:
  F6471867: KCM.png 
  
  Second issue: when adding new rows, the existing virtual desktops are 
unpredictably re-assigned. I would expect new empty rows to be created, without 
re-assigning the rows of the existing desktops.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag, ngraham
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Vlad Zagorodniy]

zzag added a comment.


  > - if any virtual desktop is removed, then System Settings window will be 
sent to the last virtual desktop.
  
  This seems to be a bug in KWin core.
  
  ---
  
  "Navigation wraps around" and the other check boxes initially don't represent 
the actual state, e.g. RollOverDesktops is set to false, but the corresponding 
checkbox is checked.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein added a subscriber: ngraham.
hein added a comment.


  It turns out we all collectively forgot about the "Switching" tab in the 
original KCM.
  
  Together with @ngraham we came up with a plan:
  
  - Drop the Shortcuts settings for now. They're duplicated from the Shortcuts 
KCM.
  - Drop the Animation setting for now. Its duplicated from the Desktop Effects 
KCM.
  - Rearrange the layout of the new KCM a bit, and add the remaining settings 
to the footer.
  
  Here's a screenshot:
  
  F6467954: Screenshot_20181210_162033.png 

  
  @ngraham would prefer to have the Animation setting in there, and I might add 
it back later in a seperate review before we go into freeze. For now, however, 
I want this reviewed and merged along with the other pending virtual desktop 
patches, so it gets testing in master. Please review.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: ngraham, davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 47240.
hein added a comment.


  Add back nav wrap and OSD settings.

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=46916&id=47240

BRANCH
  master

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Marco Martin]

mart accepted this revision.
mart added a comment.


  I've just done a round of testing of the latest revision together my last 
kwin patch, including:
  
  - adding and/or removinf desktops
  - changing the number of rows
  - renaming some desktops
  - restarting kwin randomly after any of the above to see settings are kept, 
without restarting the kcm which keeps showing coherent data
  
  so, definitely lgtm (adding ship it as my personal one, still needs David's)

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein added a comment.


  I'm no longer aware of bugs in this, please re-review it.
  
  Please have the latest version of @mart's D17265 
 applied or you may encounter weirdness 
from KWin's side.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 46916.
hein added a comment.


  Further fixes to sync & co
  
  - Adding a desktop could emit wrong model transactions (wrong container count 
was used to calculate append index).
  - `updateModifiedState` (previously `checkModifiedState`) now handles cases 
where desktop counts and names remained the same despite the user triggering 
with remove/add actions. This can happen when removing a desktop retaining the 
default name and adding a desktop back, for example. In this case the method 
will replace dummy with server ids so that the data structures match again, 
then avoid doing a sync to the server and disable the Apply button.
  - During sync, when syncing ids replace any dummy ids with server ids in the 
data structures. For cases similar to above - a desktop was replaced with an 
identically-named one, and is not handled by a remove/create -, otherwise the 
following block will emit setDesktopName D-Bus calls with invalid ids.

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=46895&id=46916

BRANCH
  master

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Vlad Zagorodniy]

zzag added a comment.


  Some issues that I saw while testing this patch:
  
  - with 2 rows and 6 virtual desktop, I get the following desktop layout (is 
it a bug in KWin core?)
  
+---+---+---+---+---+
|   |   |   |   |   |
+---+---+---+---+---+
|   |
+---+
  
  - if any virtual desktop is removed, then System Settings window will be sent 
to the last virtual desktop.

INLINE COMMENTS

> desktopsmodel.h:51
> + *
> + * After synchronization, the model tracks Kwin-side changes again,
> + * until the user makes further changes.

*KWin-side

> desktopsmodel.h:119-124
> +QStringList m_serverSideDesktops;
> +QHash m_serverSideNames;
> +int m_serverSideRows;
> +QStringList m_desktops;
> +QHash m_names;
> +int m_rows;

For better readability, you could create a struct to represent the state on 
both sides, e.g.:

  struct State
  {
  QStringList desktops;
  QHash names;
  int rows;
  };
  
  State m_clientState;
  State m_serverState;

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 46895.
hein added a comment.


  Revamp KWin restart handling
  
  The way a KWin restart is handled is now the same as the general
  "stay in sync with server if the user didn't make changes, other-
  wise stick to the user state and notify about the server-side
  change" approach:
  
  - User changes are now kept and not thrown away
  - When KWin restarts the old and the new server state are compared, and if 
the user had made any changes, the model notifies about any difference

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=46554&id=46895

BRANCH
  master

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 46554.
hein added a comment.


  Handle KWin restarts

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=46551&id=46554

BRANCH
  master

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 46551.
hein added a comment.


  Rebase on master for good measure

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=46545&id=46551

BRANCH
  master

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 46545.
hein added a comment.


  Fix syncing to server.
  
  Fixes take various shapes:
  
  - Fix bug on our side.
  - Handle weird KWin behavior, such as emiting desktopRowsChanged with an 
unchanged value.
  - s/onAccepted/onEditingFinished on the rename TextField, otherwise focus 
loss keeps the changed text but doesn't light up Apply.

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=44673&id=46545

BRANCH
  master

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Vlad Zagorodniy]

zzag added a comment.


  Other issues:
  
  - if I remove a virtual desktop and apply settings, it's not possible anymore 
to add or remove virtual desktops;
  - if I create a virtual desktop, then for some reason the KCM will try to 
create more than one:
  
  F6377804: Screenshot_20181101_225444.png 

  (it will try to reach the maximum number of virtual desktops)

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, ngraham, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 44673.
hein added a comment.


  Fix build error.

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=44672&id=44673

BRANCH
  arcpatch-D14542_2

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, ngraham, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart

D14542: [kcmkwin/desktop] KCM using new virtual desktops DBus interface

[Eike Hein]

hein updated this revision to Diff 44672.
hein retitled this revision from "KCM using new virtual desktops DBus 
interface" to "[kcmkwin/desktop] KCM using new virtual desktops DBus interface".
hein added a comment.


  Update the title.

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14542?vs=44671&id=44672

BRANCH
  arcpatch-D14542_2

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

AFFECTED FILES
  kcmkwin/kwindesktop/CMakeLists.txt
  kcmkwin/kwindesktop/Messages.sh
  kcmkwin/kwindesktop/desktop.desktop
  kcmkwin/kwindesktop/desktopnameswidget.cpp
  kcmkwin/kwindesktop/desktopnameswidget.h
  kcmkwin/kwindesktop/desktopsmodel.cpp
  kcmkwin/kwindesktop/desktopsmodel.h
  kcmkwin/kwindesktop/kcm_kwin_virtualdesktops.desktop
  kcmkwin/kwindesktop/main.cpp
  kcmkwin/kwindesktop/main.h
  kcmkwin/kwindesktop/main.ui
  kcmkwin/kwindesktop/package/contents/ui/main.qml
  kcmkwin/kwindesktop/package/metadata.desktop
  kcmkwin/kwindesktop/virtualdesktops.cpp
  kcmkwin/kwindesktop/virtualdesktops.h

To: hein, mart, davidedmundson, ltoscano, zzag
Cc: davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, 
jackyalcine, Pitel, iodelay, bwowk, ZrenBot, ngraham, lesliezhai, ali-mohamed, 
hardening, jensreuterberg, abetts, sebas, apol, mart