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

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=47750

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

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

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

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

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

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

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

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

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

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,

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

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

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

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,

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

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

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

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 .

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

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,

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

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=47546 BRANCH master REVISION DETAIL https://phabricator.kde.org/D14542 AFFECTED FILES

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

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,

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

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.

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

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

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

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.

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

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

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

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=47240 BRANCH master REVISION DETAIL https://phabricator.kde.org/D14542 AFFECTED FILES

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

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

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

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

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

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

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

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

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

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

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

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=46554 BRANCH master REVISION DETAIL https://phabricator.kde.org/D14542 AFFECTED FILES

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

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=46551 BRANCH master REVISION DETAIL https://phabricator.kde.org/D14542 AFFECTED FILES

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

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

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

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:

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

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=44673 BRANCH arcpatch-D14542_2 REVISION DETAIL https://phabricator.kde.org/D14542 AFFECTED FILES

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

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