[Notifications] Allow selecting text with mouse
ClosedPublic

Authored by broulik on Oct 24 2017, 12:52 PM.

Details

Summary

This was lost by the introduction of the default action as the MouseArea steals events from the TextField below.
To address this, selecting text and click handling has been implemented manually. Furthermore, the mouse cursor is changed to an IBeam cursor and turns into a hand when above hyperlinks whose URL can now also be copied separately from its text in the context menu.

BUG: 386118
FIXED-IN: 5.12.0

Test Plan
  • Clicking action buttons still works
  • Clicking a notification with default action will still trigger the default action. An action without will not do anything, ie. the dialog won't close, just as before
  • Clicking a hyperlink will open it
  • Click vs select drag feels natural, I implemented it using manhattan length similarly to MouseEventListener
  • The cursor is an IBeam over the entire body text area and turns into a pointing hand when ontop of a hyperlink
  • Right click menu "Select All" entries select all text, "Copy" copies the text. Ontop of Hyperlinks the "Copy Link" entry copies the hyperlink's URL
  • Notification still seems to timeout properly, with or without user interaction, but stays open while cursor is ontop of it

Needs D8445 (ungrab mouse hack) or else after having opened the context menu, you continue randomly selecting text as the MouseArea gets confused as to whether the mouse is pressed or not.

Only quirk I noticed is that while selecting text will scroll down the view as needed it won't work to scroll back up but whatever.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Oct 24 2017, 12:52 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 24 2017, 12:52 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik edited the test plan for this revision. (Show Details)Oct 24 2017, 12:53 PM
apol added a comment.Oct 24 2017, 1:46 PM

I don't think selecting stuff from a notification should be a use case. It means that the notification is poorly designed. The code shows.
That said, you are the maintainer.

As for the code, there's no way I can make sense out of this code, I believe you that it works.

+1 IMHO, any text anywhere should always be selectable. You never know when or why the user might want or need to select some text on the system, so this makes sense to me at least.

apol added a comment.Oct 24 2017, 3:26 PM

+1 IMHO, any text anywhere should always be selectable. You never know when or why the user might want or need to select some text on the system, so this makes sense to me at least.

Such generic statements can't make sense. Do you want to add text selection on window decoration, the clocks, kickoff?
Note that this requires adding tons of complex (and brittle) code such as this patch.

I don't think selecting stuff from a notification should be a use case.

I occasionally do. Just yesterday I received an SMS and instead of being able to copy something from it I had to grab my phone and send the link to my computer instead.

This revision was automatically updated to reflect the committed changes.