D17775: [kcmkwin/kwindecoration] Set current decoration index on start

davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kcm.cpp:109
>      m_quickView->rootContext()->setContextProperty("availableButtons", m_availableButtons);
> +    m_quickView->rootContext()->setContextProperty("savedIndex", 0);
>  

This is done on line 317.

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

> kcm.cpp:319
>  
> +    if (auto listView = m_quickView->rootObject()->findChild<QQuickItem*>("listView")) {
> +        listView->setProperty("currentIndex", themeIndex);

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

> Previews.qml:38
> -            }
> -            gridView.positionViewAtIndex(gridView.currentIndex, GridView.Visible);
> -        }

This line is still relevant

> Previews.qml:64
>                  onClicked: {
> -                    gridView.currentIndex = index;
> +                    if (gridView.currentIndex != index) {
> +                        gridView.currentIndex = index;

why this?

REPOSITORY
  R108 KWin

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

To: vpilo, davidedmundson
Cc: davidedmundson, kwin, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart