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.

> broulik wrote in osd.cpp:139
> This assert can never be hit, you *always* create the object or return early 
> and never end up here

That is correct. Do you prefer not to have the assert?

> broulik wrote in OsdSelector.qml:31
> Why initially visible?

It doesn't matter, can happily be removed.

> broulik wrote in OsdSelector.qml:117
> This looks unrelated to this particular patch ("In follow up changes, the 
> keyboard handling will be added.")

True, I can take it out, it doesn't work anyway since we don't the focus into 
the dialog anyway.

REPOSITORY
  R104 KScreen

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

To: gladhorn, #plasma, davidedmundson
Cc: broulik, davidedmundson, zzag, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

Reply via email to