Change cursor shape to pointing hand during mouse hover event and copy contents of display to clipboard on mouse press event
Needs ReviewPublic

Authored by shubham on Jul 23 2019, 10:56 AM.

Details

Reviewers
cfeck
teran
ngraham
Group Reviewers
VDG
Summary

Before:
Mouse click over display area used to result in black screen display for a split second (Left click). It looked ugly and useless.

Now:
Hovering over the display area changes the Cursor shape to a pointing hand.
Nothing happens on mouse click over display area (Copy to clipboard still works).

Test Plan

Try to mouse click over display area. Mouse cursor changes to a pointing hand indicating it does "something" (Copy to clipboard still works). Nothing unwanted happens .

Diff Detail

Repository
R353 KCalc
Branch
color
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14348
Build 14366: arc lint + arc unit
shubham created this revision.Jul 23 2019, 10:56 AM
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptJul 23 2019, 10:56 AM
shubham requested review of this revision.Jul 23 2019, 10:56 AM
shubham edited the summary of this revision. (Show Details)Jul 23 2019, 10:59 AM
shubham edited the test plan for this revision. (Show Details)
shubham added reviewers: cfeck, teran.
shubham edited the summary of this revision. (Show Details)Jul 23 2019, 3:20 PM
ngraham requested changes to this revision.EditedJul 23 2019, 4:13 PM

I agree that the current UX is not ideal. However you're just removing features without understanding what they do. Left-clicking copies the number to the clipboard. I don't know what right-clicking does, but we need to understand it before we remove it.

For the left-click case, I would recommend showing a label or low-priority notification that informs the user that the text has been copied to the clipboard. Also, changing the cursor to be a pointing finger while hovered over the display would be good too, so people get an idea that it's clickable. Then we can probably remove the color inverting effect.

This revision now requires changes to proceed.Jul 23 2019, 4:13 PM
shubham updated this revision to Diff 62435.EditedJul 23 2019, 7:37 PM

Made requested changes

I need help adding notifications. They are still not shown.

shubham edited the summary of this revision. (Show Details)Jul 23 2019, 7:38 PM
shubham edited the test plan for this revision. (Show Details)
shubham edited the summary of this revision. (Show Details)
ngraham requested changes to this revision.Jul 23 2019, 7:48 PM
ngraham added a subscriber: broulik.

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

kcalc.notifyrc
3

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

9

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

11

Add Urgency=Low

kcalcdisplay.cpp
60–61

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

365

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

366

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? :)

This revision now requires changes to proceed.Jul 23 2019, 7:48 PM
shubham updated this revision to Diff 62436.Jul 23 2019, 8:05 PM

made some more changes

shubham marked 5 inline comments as done.Jul 23 2019, 8:06 PM
shubham added inline comments.
kcalcdisplay.cpp
366

I was just playing around to see if something works.

shubham updated this revision to Diff 62437.Jul 23 2019, 8:09 PM

Simplify much more

shubham marked an inline comment as done.Jul 23 2019, 8:09 PM
cfeck added a comment.Jul 24 2019, 1:22 PM

I don't think a notification is useful here. Note that a notification is displayed at a complete different location. The feedback that you actually clicked the number to copied it should be visible there.

I don't think a notification is useful here. Note that a notification is displayed at a complete different location. The feedback that you actually clicked the number to copied it should be visible there.

Yeah, if this was a Kirigami app, I'd recommend making use of the PassiveNotification feature for this. it's exactly the kind of use case that the feature was designed for. Unfortunately, it's not, so we can't use that.

Need to update the title and description to reflect the current state of the patch.

shubham retitled this revision from Remove code which resulted useless action to Change cursor shape to pointing hand during mouse hover event and copy contents of display to clipboard on mouse press event.Aug 24 2019, 3:05 PM
shubham edited the test plan for this revision. (Show Details)
ngraham added a reviewer: VDG.Dec 13 2019, 8:41 PM

Sorry I missed this. Even though I recommended using a system notification to show that the clipboard has been copied, in retrospect I kind of don't like it because it feels a bit weird and intrusive. Kirigami's PassiveNotification would be better, but this isn't a QML app, and I can't think of anything better right now. Adding VDG for further comment.

I guess you could use an automatically disappearing KMessageWidget to show the "copied to clipboard" notification. Spectacle does this; you could use that for inspiration maybe?