[kcmkwin/kwindecoration] Set current decoration index on start
ClosedPublic

Authored by vpilo on Dec 24 2018, 3:26 PM.

Details

Summary

Fixes the preselected decoration style on module load.

Also:

  • Prevent the module state to be set to modified on resize.
  • Fix QML errors in logs.

Diff Detail

Repository
R108 KWin
Branch
vpilo/CurrentDecoration
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6596
Build 6614: arc lint + arc unit
vpilo created this revision.Dec 24 2018, 3:26 PM
Restricted Application added a project: KWin. · View Herald TranscriptDec 24 2018, 3:26 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
vpilo requested review of this revision.Dec 24 2018, 3:26 PM
davidedmundson requested changes to this revision.Dec 24 2018, 10:51 PM
davidedmundson added a subscriber: davidedmundson.
davidedmundson added inline comments.
kcmkwin/kwindecoration/kcm.cpp
109

This is done on line 317.

What's the advantage of doing it here?
(Also after this patch who uses savedIndex ?)

319

Creating delegates in a grid view is async from setting the model.

Setting the curentIndex before the delegate is created is the bug that the existing code is trying to fix.

(even though I agree the current solution is quite wrong)

A common pattern is to update the grid index inside the delegate constructor

kcmkwin/kwindecoration/qml/Previews.qml
38

This line is still relevant

76–77

why this?

This revision now requires changes to proceed.Dec 24 2018, 10:51 PM
vpilo marked an inline comment as done.Jan 3 2019, 1:45 AM
vpilo added inline comments.
kcmkwin/kwindecoration/kcm.cpp
109

It's to prevent "undefined reference" errors in the qml. I could have used typeof() != "undefined" in the QML but this is cleaner

319

I rewrote the whole thing, thank you for the tip!

kcmkwin/kwindecoration/qml/Previews.qml
76–77

Leftover from my initial investigation

vpilo updated this revision to Diff 48584.Jan 3 2019, 1:47 AM

Rewrote

davidedmundson accepted this revision.Jan 3 2019, 10:22 AM
This revision is now accepted and ready to land.Jan 3 2019, 10:22 AM

Do you have commit access?

vpilo added a comment.EditedJan 3 2019, 10:26 AM

Do you have commit access?

Yep :) I just am not able to reach anongit.kde.org for fetching, is that a known issue at the moment?

This revision was automatically updated to reflect the committed changes.
ngraham added a subscriber: ngraham.Jan 7 2019, 2:46 AM

BTW this patch doesn't fully fix https://bugs.kde.org/show_bug.cgi?id=397595 for me. The active decoration theme is now correctly selected when opening the KCM standalone in kcmshell5, but it's still unselected from within System Settings itself.