Differential D7690

Add "Open in New Window to Dolphin's places context menu
ClosedPublic
Actions

Authored by ngraham on Sep 4 2017, 5:37 PM.

Details

Reviewers
dfaure
aacid
elvisangelaccio
emmanuelp
broulik
Group Reviewers
Dolphin
KDE Applications
Commits
R318:2b2bab4049d9: Add "Open in New Window to Dolphin's places context menu
Summary

BUG: 384352

Add an "Open in New Window" menu item to the context menu for Dolphin's places items.

Test Plan

Tested in up-to-date KDE Neon. Works as expected:

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Sep 4 2017, 5:37 PM
Restricted Application added a subscriber: Dolphin. · View Herald TranscriptSep 4 2017, 5:37 PM
ngraham edited the test plan for this revision. (Show Details)Sep 4 2017, 5:40 PM
ngraham added reviewers: Dolphin, KDE Applications, dfaure, aacid, elvisangelaccio.
ngraham added a project: Dolphin.
broulik added a subscriber: broulik.Sep 6 2017, 8:50 AM
broulik added inline comments.
src/panels/places/placespanel.cpp
238

Why do you need upUrl here?

emmanuelp requested changes to this revision.Sep 6 2017, 9:18 PM
emmanuelp added a subscriber: emmanuelp.
emmanuelp added inline comments.
src/panels/places/placespanel.cpp
238

Please use PlacesItemModel::convertedUrl(m_model->data(index).value("url").toUrl()) instead

This revision now requires changes to proceed.Sep 6 2017, 9:18 PM
ngraham updated this revision to Diff 19258.Sep 7 2017, 3:21 AM
ngraham edited edge metadata.

Made the requested changes. Tested again; still works.

broulik accepted this revision.Sep 8 2017, 10:40 PM
ngraham marked 2 inline comments as done.Sep 11 2017, 2:08 AM

@emmanuelp Does this look commitable now?

src/panels/places/placespanel.cpp
238

I was just copying code I found elsewhere in Dolphin to do this. I've made the requested change.

emmanuelp accepted this revision.Sep 11 2017, 2:19 PM
In D7690#144674, @ngraham wrote:

@emmanuelp Does this look commitable now?

Yes, nice work. Thanks!

This revision is now accepted and ready to land.Sep 11 2017, 2:19 PM
ngraham marked an inline comment as done.Sep 11 2017, 2:21 PM

Thank you! I don't have commit access, so someone else will need to do it, I'm afraid.

Closed by commit R318:2b2bab4049d9: Add "Open in New Window to Dolphin's places context menu (authored by ngraham, committed by aacid). · Explain WhySep 11 2017, 8:04 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/panels/places/placespanel.cpp (4 lines)

Diff 19419

View Options

src/panels/places/placespanel.cpp

Loading...