D28517: Send notification when screencasting fails

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> screencaststream.cpp:372
> +    const auto emitFailureNotification = [](const QString &body) {
> +        KNotification *notification = new KNotification(QStringLiteral("notification"), KNotification::CloseOnTimeout | KNotification::DefaultEvent);
> +        notification->setTitle(i18n("Failed to start screencasting"));

This should be its own event, not abusing the "notification" one

> screencaststream.cpp:375
> +        notification->setText(i18nc("Introduces an error message", "Error: %1", body));
> +        notification->setIconName(QStringLiteral("data-error"));
> +        notification->setUrgency(KNotification::HighUrgency);

You probably want `dialog-error`

> screencaststream.cpp:376
> +        notification->setIconName(QStringLiteral("data-error"));
> +        notification->setUrgency(KNotification::HighUrgency);
> +        notification->sendEvent();

You sure this is needed to be high?

> ngraham wrote in screencaststream.cpp:374
> `.arg` is unnecessary and in various cases can cause i18n() errors; just do it inline: `notification->setText(i18n("Error: %1", body));`
> 
> Also for short strings like this, it's nice to add some context for translators.

Is the "Error" even needed?

REPOSITORY
  R838 Flatpak Support: KDE Portal for XDG Desktop

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

To: cblack, #plasma, #vdg, ngraham, apol
Cc: broulik, apol, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200402/00650587/attachment.html>