kcmshell5 kcm_kwin_virtualdesktops
kcm works as before
Details
- Reviewers
ervin crossi zzag - Group Reviewers
KWin - Commits
- R108:790e717c82cb: [KCM/Desktop] Port from KQuickAddons::ConfigModule to ManagedConfigModule
Diff Detail
- Repository
- R108 KWin
- Branch
- use-kconfigxt-in-qml
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 19692 Build 19710: arc lint + arc unit
kcmkwin/kwindesktop/virtualdesktops.cpp | ||
---|---|---|
163 | It is needed m_animationsModel and m_desktopsModel are not handled by ManagedConfigModule (they are not KCoreConfigSkeleton but QAbstractItemModel and QAbstractListModel). |
kcmkwin/kwindesktop/virtualdesktops.cpp | ||
---|---|---|
163 | This is fragile still, please try to override isDefaults and isSaveNeeded instead. |
Fix AnimationModel and DesktopModel to have defaults/isDefault methods, add isDefaults and isSaveNeeded to VirtualDesktops, use onToggled in qml
Much better with isDefaults and isSaveNeeded override.
kcmkwin/kwindesktop/virtualdesktops.cpp | ||
---|---|---|
175 | Could provide more simpler and nicer implementation like isDefaults() above, something like : return m_desktopsModel->needsSave() || m_animationsModel->needsSave(); |
kcmkwin/kwindesktop/virtualdesktops.cpp | ||
---|---|---|
175 | I think we can simplify the whole method by return m_desktopsModel->needsSave() || m_animationsModel->needsSave(); |
Lower case first letter of entries names in kcfg, fix immutable state read for rollOverDesktops
kcmkwin/kwindesktop/desktopsmodel.cpp | ||
---|---|---|
379 ↗ | (On Diff #71705) | Put the opening brace on the next line. |
This patch looks good to me.
kcmkwin/kwindesktop/animationsmodel.cpp | ||
---|---|---|
140 ↗ | (On Diff #71755) | Heh, it took a couple of minutes to decipher what this method is doing. Could you please add an explanatory comment why it's okay to check only EnabledByDefaultRole and not StatusRole? |
Just a minor stylistic issue I missed the last time
kcmkwin/kwindesktop/animationsmodel.cpp | ||
---|---|---|
141 ↗ | (On Diff #71899) | Missing space after the comma. |