D22679: Remove code which resulted useless action

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a subscriber: broulik.
ngraham added a comment.
This revision now requires changes to proceed.


  Perhaps @broulik can help you figure out why the notification isn't appearing.

INLINE COMMENTS

> kcalc.notifyrc:2
> +[Global]
> +IconName=Filename
> +Name=kcalc 

This should be the app's own icon (i.e. `kcalc`)

> kcalc.notifyrc:8
> +Name=Text Copied
> +Comment=Text has been copied to clipboard
> +Action=Popup

From a user perspective, it's not text, it's the number

> kcalc.notifyrc:10
> +Action=Popup
> +

Add `Urgency=Low`

> kcalcdisplay.cpp:61
>  	connect(this, &KCalcDisplay::clicked, this, &KCalcDisplay::slotDisplaySelected);
> -	connect(selection_timer_, &QTimer::timeout, this, &KCalcDisplay::slotSelectionTimedOut);
> +	//connect(selection_timer_, &QTimer::timeout, this, &KCalcDisplay::slotSelectionTimedOut);
>  

Don't commit commented-out code. Remove if it's not used anymore.

> kcalcdisplay.cpp:366
> +        KNotification *textCopied = new KNotification(QStringLiteral("Mouse Clicked"), KNotification::CloseOnTimeout, nullptr);
> +        textCopied->setText(QStringLiteral("Text copied to clipboard"));
> +        textCopied->setUrgency(KNotification::HighUrgency);

You already defined the text in the notifyrc file; don't set it again here

> kcalcdisplay.cpp:367
> +        textCopied->setText(QStringLiteral("Text copied to clipboard"));
> +        textCopied->setUrgency(KNotification::HighUrgency);
> +        textCopied->sendEvent();

How could this possibly be a high urgency notification? What disaster befalls the user if they don't see a notification that text has been copied to the clipboard? :)

REPOSITORY
  R353 KCalc

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

To: shubham, cfeck, teran, ngraham
Cc: broulik, kde-utils-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20190723/de2ad0a5/attachment.html>