D14143: Make the action selector OSD independent of the other OSDs

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R104:5238ae3a7251: Make the action selector OSD independent of the other OSDs (authored by gladhorn). CHANGED PRIOR TO COMM

D14143: Make the action selector OSD independent of the other OSDs

gladhorn added a comment. I'd like to get this in since the meaningful changes (keyboard handling) are based on this :) REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D14143 To: gladhorn, #plasma, davidedmundson Cc: broulik, davidedmundson, zzag, plasma-devel, ragre

D14143: Make the action selector OSD independent of the other OSDs

gladhorn updated this revision to Diff 38117. gladhorn added a comment. Use QmlObjectSharedEngine REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14143?vs=38029&id=38117 BRANCH arcpatch-D14143_1 REVISION DETAIL https://phabricator.kde.org/D14143 AFFEC

D14143: Make the action selector OSD independent of the other OSDs

gladhorn updated this revision to Diff 38029. gladhorn marked 4 inline comments as done. gladhorn added a comment. Do not yet introduce any keyboard handling (escape snuck in) and remove clutter (assert, visible:true). REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator

D14143: Make the action selector OSD independent of the other OSDs

gladhorn added inline comments. INLINE COMMENTS > broulik wrote in osd.cpp:125 > Should we use `QmlObjectSharedEngine` here? (could be done separately later) I think the whole OSD class is constantly being deleted/re-created (after 5 seconds of not being used iirc) so for now this is moot. > b

D14143: Make the action selector OSD independent of the other OSDs

broulik added inline comments. INLINE COMMENTS > osd.cpp:125 > +} > +m_osdActionSelector = new KDeclarative::QmlObject(this); > +m_osdActionSelector->setSource(QUrl::fromLocalFile(osdPath)); Should we use `QmlObjectSharedEngine` here? (could be done separately later) > o

D14143: Make the action selector OSD independent of the other OSDs

gladhorn added inline comments. INLINE COMMENTS > davidedmundson wrote in osd.cpp:144 > I don't understand how this hideOsd() will work, it only updates m_osdObject > which is the other OSD. Ouch, there you saved me :) osdtest works since it exits, with the real osd through kded it doesn't wor

D14143: Make the action selector OSD independent of the other OSDs

gladhorn updated this revision to Diff 37994. gladhorn marked 2 inline comments as done. gladhorn added a comment. Fix hiding of osds REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14143?vs=37984&id=37994 BRANCH arcpatch-D14143 REVISION DETAIL https:/

D14143: Make the action selector OSD independent of the other OSDs

davidedmundson requested changes to this revision. davidedmundson added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > osd.cpp:144 > Q_EMIT osdActionSelected(static_cast(action)); > hideOsd(); > } I don't understand how this hideOsd() will work, it o

D14143: Make the action selector OSD independent of the other OSDs

gladhorn updated this revision to Diff 37984. gladhorn added a comment. return qstring without ref REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14143?vs=37923&id=37984 BRANCH arcpatch-D14143_2 REVISION DETAIL https://phabricator.kde.org/D14143 AFFE

D14143: Make the action selector OSD independent of the other OSDs

gladhorn updated this revision to Diff 37923. gladhorn added a comment. re-added const REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14143?vs=37850&id=37923 BRANCH arcpatch-D14143_1 REVISION DETAIL https://phabricator.kde.org/D14143 AFFECTED FILES

D14143: Make the action selector OSD independent of the other OSDs

gladhorn updated this revision to Diff 37850. gladhorn added a comment. Remove string ref in return. REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14143?vs=37826&id=37850 BRANCH arcpatch-D14143 REVISION DETAIL https://phabricator.kde.org/D14143 AFFE

D14143: Make the action selector OSD independent of the other OSDs

gladhorn added inline comments. INLINE COMMENTS > zzag wrote in osd.cpp:120 > s/expansion/extension/ Good point. I copied this from the original instantiation, will fix both. REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D14143 To: gladhorn, #plasma Cc: zzag, plasma-

D14143: Make the action selector OSD independent of the other OSDs

zzag added inline comments. INLINE COMMENTS > zzag wrote in osd.cpp:120 > Please delete reference. Compilers can eliminate unnecessary copying, so > there is no need to do the lifetime expansion. s/expansion/extension/ REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D1

D14143: Make the action selector OSD independent of the other OSDs

zzag added inline comments. INLINE COMMENTS > osd.cpp:120 > +if (!m_osdActionSelector) { > +const QString &osdPath = > QStandardPaths::locate(QStandardPaths::QStandardPaths::GenericDataLocation, > QStringLiteral("kded_kscreen/qml/OsdSelector.qml")); > +if (osdPath.isEmpty())

D14143: Make the action selector OSD independent of the other OSDs

gladhorn created this revision. gladhorn added a reviewer: Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. gladhorn requested review of this revision. REVISION SUMMARY This OSD should handle keyboard input and behave different, so