Differential D2392

[Task Manager] Allow dropping files onto tasks
ClosedPublic
Actions

Authored by broulik on Aug 10 2016, 10:37 AM.

Details

Reviewers
hein
Group Reviewers
Plasma: Design
Plasma
Commits
R119:56a21fe0ec12: [Task Manager] Allow dropping files onto tasks
Summary

This allows to drop files onto entries in the task manager. Both single and multiple files are supported.

When all dropped items are desktop files for applications, such as dragging apps from Kickoff, a launcher is created for each item.
Otherwise the application is launched with those URLs as arguments and behavior depends on the application. It might open the file(s) in new tab(s) of an existing instance, open a new window with tabs, open new windows for each files, etc.

BUG: 365626
FIXED-IN: 5.8.0

Test Plan

See video: https://www.youtube.com/watch?v=WHx5J_W4fqo
Depends on https://phabricator.kde.org/D2391

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik updated this revision to Diff 5794.Aug 10 2016, 10:37 AM
broulik retitled this revision from to [Task Manager] Allow dropping files onto tasks.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik added reviewers: Plasma, Plasma: Design.
broulik set the repository for this revision to R119 Plasma Desktop.
Restricted Application added a project: Plasma. · View Herald TranscriptAug 10 2016, 10:37 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik updated this object.Aug 10 2016, 10:38 AM
hein added a subscriber: hein.Aug 12 2016, 9:32 AM
hein added inline comments.
applets/taskmanager/package/contents/ui/main.qml
247 ↗(On Diff #5794)

I know this is nitpicky, but could you make sure code comments are generally proper sentences with correct capitalization and punctuation? Errors are errors, and I don't want to have to flag them all the time since it costs "review energy" to feel bad for being nitpicky.

250 ↗(On Diff #5794)

Does JS have anything like Python any/all we could use to avoid loop boilerplate?

268 ↗(On Diff #5794)

Experience around the messyness of DND in Qt Quick tells me you want to add a check on hoveredItem :)

applets/taskmanager/plugin/backend.cpp
346 ↗(On Diff #5794)

QUrl::isValid() famously won't return false for URLs that are actually invalid in strict mode but were constructed in tolerant mode, so trusting it for QUrls you didn't construct it is a bad idea. This probably doesn't matter here because I hope KRun checks, but it's worth mentioning in case you want to be more paranoid.

350 ↗(On Diff #5794)

This could be a const ref ... even if it's implicitly shared ...

broulik updated this revision to Diff 5865.Aug 12 2016, 9:54 AM
  • Fix comments punctuation
  • Use JS every() and forEach() instead of hand-rolled loops
  • Check hoveredItem before accessing it

Did not change the QUrl validation as that's been like this before and I don't know how to "properly" do it

hein accepted this revision.Aug 12 2016, 9:58 AM
hein added a reviewer: hein.
This revision is now accepted and ready to land.Aug 12 2016, 9:58 AM
hein accepted this revision.Aug 12 2016, 9:58 AM
Closed by commit R119:56a21fe0ec12: [Task Manager] Allow dropping files onto tasks (authored by broulik). · Explain WhyAug 12 2016, 10:46 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 5865

View Options

applets/taskmanager/package/contents/ui/MouseHandler.qml

Loading...
View Options

applets/taskmanager/package/contents/ui/main.qml

Loading...
View Options

applets/taskmanager/plugin/backend.h

Loading...
View Options

applets/taskmanager/plugin/backend.cpp

Loading...