D17416: [Effects KCM] Use a better selection color, fix list item size after hiding the effect video and use a real button the play button

[Björn Feber]

GB_2 created this revision.
GB_2 added reviewers: KWin, VDG.
GB_2 added projects: KWin, VDG.
Herald added a subscriber: kwin.
GB_2 requested review of this revision.

REVISION SUMMARY
Makes some improvements to the Effects KCM (details in title).
F6461903: Effects.png <https://phabricator.kde.org/F6461903>

TEST PLAN
Open the Effects KCM.

REPOSITORY
R108 KWin

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

AFFECTED FILES
kcmkwin/kwincompositing/qml/Effect.qml
kcmkwin/kwincompositing/qml/EffectView.qml
kcmkwin/kwincompositing/qml/Video.qml

To: GB_2, #kwin, #vdg
Cc: #vdg, kwin, #kwin, alexde, IohannesPetros, mkulinski, trickyricky26, ragreen, jackyalcine, Pitel, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, skadinna, lesliezhai, ali-mohamed, hardening, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart

[Nathaniel Graham]

ngraham added a comment.


Cool!

Should these list items even become selected when clicked? Typically items are only selectable when there are some actions you can apply to selected items, which isn't the case here.

Also, since this seems to be an open-ended "improve the Effects KCM" patch, I might recommend showing the video in a tastefully constructed pop-up rather than expanding the height of the corresponding list item to accommodate the video, which just doesn't look good.

REPOSITORY
R108 KWin

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

To: GB_2, #kwin, #vdg
Cc: ngraham, #vdg, kwin, #kwin, alexde, IohannesPetros, mkulinski, trickyricky26, ragreen, jackyalcine, Pitel, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, skadinna, lesliezhai, ali-mohamed, hardening, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart

[Björn Feber]

GB_2 retitled this revision from "[Effects KCM] Use a better selection color, fix list item size after hiding the effect video and use a real button the play button" to "[Effects KCM] Use a better selection color, fix list item size after hiding the effect video and use a real button as the play button".

REPOSITORY
R108 KWin

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

To: GB_2, #kwin, #vdg
Cc: ngraham, #vdg, kwin, #kwin, alexde, IohannesPetros, mkulinski, trickyricky26, ragreen, jackyalcine, Pitel, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, skadinna, lesliezhai, ali-mohamed, hardening, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart

[Björn Feber]

GB_2 updated this revision to Diff 47117.
GB_2 edited the summary of this revision.
GB_2 added a comment.


Remove effect list item selection and use a Breeze BusyIndicator.
Opening a new dialog for the video would need some more work, so it's not included here.

REPOSITORY
R108 KWin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17416?vs=47064&id=47117

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

AFFECTED FILES
kcmkwin/kwincompositing/qml/Effect.qml
kcmkwin/kwincompositing/qml/EffectView.qml
kcmkwin/kwincompositing/qml/Video.qml

To: GB_2, #kwin, #vdg
Cc: ngraham, #vdg, kwin, #kwin, alexde, IohannesPetros, mkulinski, trickyricky26, ragreen, jackyalcine, Pitel, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, skadinna, lesliezhai, ali-mohamed, hardening, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart

[Nathaniel Graham]

ngraham added a comment.


Hmm, why did you change the busy indicator? I think the current one is fine. The new one is something I haven't seen used anywhere else.

REPOSITORY
R108 KWin

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

To: GB_2, #kwin, #vdg
Cc: ngraham, #vdg, kwin, #kwin, alexde, IohannesPetros, mkulinski, trickyricky26, ragreen, jackyalcine, Pitel, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, skadinna, lesliezhai, ali-mohamed, hardening, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart

[Björn Feber]

GB_2 added a comment.

Post by Nathaniel Graham
Hmm, why did you change the busy indicator? I think the current one is fine. The new one is something I haven't seen used anywhere else.

I think the default QML busy indicator looks bad and doesn't really fit in Breeze.
What other QML busy indicator could be used?

REPOSITORY
R108 KWin

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

To: GB_2, #kwin, #vdg
Cc: ngraham, #vdg, kwin, #kwin, alexde, IohannesPetros, mkulinski, trickyricky26, ragreen, jackyalcine, Pitel, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, skadinna, lesliezhai, ali-mohamed, hardening, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart

[Björn Feber]

GB_2 updated this revision to Diff 47136.
GB_2 retitled this revision from "[Effects KCM] Use a better selection color, fix list item size after hiding the effect video and use a real button as the play button" to "[Effects KCM] Use a better selection color, fix list item size after hiding the effect video, use a real button as the play button and use the right busy indicator".
GB_2 edited the summary of this revision.
GB_2 added a comment.


Use the right busy indicator.
F6464355: Effects (2).png <https://phabricator.kde.org/F6464355>

REPOSITORY
R108 KWin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17416?vs=47117&id=47136

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

AFFECTED FILES
kcmkwin/kwincompositing/qml/Effect.qml
kcmkwin/kwincompositing/qml/EffectView.qml
kcmkwin/kwincompositing/qml/Video.qml

To: GB_2, #kwin, #vdg
Cc: ngraham, #vdg, kwin, #kwin, alexde, IohannesPetros, mkulinski, trickyricky26, ragreen, jackyalcine, Pitel, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, skadinna, lesliezhai, ali-mohamed, hardening, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart

[Björn Feber]

GB_2 retitled this revision from "[Effects KCM] Use a better selection color, fix list item size after hiding the effect video, use a real button as the play button and use the right busy indicator" to "[Effects KCM] Remove effect list item selection, fix list item size after hiding the effect video, use a real button as the play button and use the right busy indicator".

REPOSITORY
R108 KWin

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

To: GB_2, #kwin, #vdg
Cc: ngraham, #vdg, kwin, #kwin, alexde, IohannesPetros, mkulinski, trickyricky26, ragreen, jackyalcine, Pitel, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, skadinna, lesliezhai, ali-mohamed, hardening, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, mbohlender, mart