D17766: [kcmkwin/desktop] Add animation option back

davidedmundson added a comment.


  Two minor things, but in general ship it!

INLINE COMMENTS

> main.qml:222
> +                    currentIndex: kcm.animationsModel.currentIndex
> +                    onCurrentIndexChanged: kcm.animationsModel.currentIndex = currentIndex
> +                }

Generally 
onActivated

is better as that is only user changes.

onCurrentIndexChanges has quirky issues if you ever make that model load asyncronously or whatever as it constantly rebounds.
Might not occur right now, but worth future proofing.

> virtualdesktops.cpp:241
> +    } else {
> +        kcm->load();
> +    }

I assume you've seen https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0
as you're using the QPointer on the exec'ing dialog, but you need to check here in the branch where dialog is rejected.

REPOSITORY
  R108 KWin

BRANCH
  extract-effects-model

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

To: zzag, #kwin, #vdg, ngraham
Cc: davidedmundson, hein, ngraham, kwin, kvanton, jraleigh, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart