Discussion:
D15929: Add a 'Propreties' entry in the Places panel context menu
Thomas Surrel
2018-10-03 20:23:31 UTC
Permalink
thsurrel created this revision.
thsurrel added reviewers: Dolphin, Plasma, VDG.
Herald added a project: Dolphin.
Herald added a subscriber: kfm-devel.
thsurrel requested review of this revision.

TEST PLAN
Right-click on a place or on a device in the Places panel now show a Properties entry.
Clicking on it will open its folder property.

REPOSITORY
R318 Dolphin

BRANCH
arc_properties (branched from master)

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

AFFECTED FILES
src/panels/places/placesitemmodel.cpp
src/panels/places/placesitemmodel.h
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg
Cc: kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-03 20:24:03 UTC
Permalink
thsurrel added a comment.


In that context menu, the 'Edit' entry (when righ-clicking on a 'Place', not on a 'Device') uses the same icon that is elsewhere used for 'Properties'. Could you suggest what to do here, #VDG <https://phabricator.kde.org/tag/vdg/> ?

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Andres Betts
2018-10-03 20:26:24 UTC
Permalink
abetts added a comment.


Can you please share a screenshot or before/after image that shows your changes?

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-03 20:35:46 UTC
Permalink
thsurrel edited the summary of this revision.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-03 20:36:39 UTC
Permalink
thsurrel added a comment.


F6301766: before.png <https://phabricator.kde.org/F6301766>

F6301770: after.png <https://phabricator.kde.org/F6301770>

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Andres Betts
2018-10-03 20:38:57 UTC
Permalink
abetts added a comment.


I would not be opposed to this change. Any other thoughts?

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Nathaniel Graham
2018-10-03 20:44:06 UTC
Permalink
ngraham added a comment.


This fixes an ancient bug and plugs an obvious hole (you right-click on a disk and then wonder "Where's the properties menu item?". +1. Will review in detail later today.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Nathaniel Graham
2018-10-03 20:44:19 UTC
Permalink
ngraham retitled this revision from "Add a 'Propreties' entry in the Places panel context menu" to "Add a 'Properties' entry in the Places panel context menu".

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-03 20:54:43 UTC
Permalink
thsurrel added a comment.


Note that I don't know if it is a good idea to add this 'Properties' entry to Places in addition to Devices.
Maybe it is useful to Devices alone. Plus it adds this problem of identical icon with Edit, as pointed in my first comment.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Nathaniel Graham
2018-10-03 21:02:11 UTC
Permalink
ngraham added a comment.


I would not be opposed to adding a Properties menu item for other Places panel items, too. But you're right: the Edit... menu item is using the wrong icon. It should be changed (in another patch) to `edit-entry`.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Nathaniel Graham
2018-10-03 21:08:00 UTC
Permalink
ngraham added a comment.


One more thing before I do the formal review: KIO also has Places Panel code. Could this be done there so we don't have code duplication? Or if code duplication is unfortunately necessary, could we get (in another patch obviously) the same feature added to the KIO Places Panel implementation too?

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-03 21:13:42 UTC
Permalink
thsurrel added a comment.


Now you are talking about something I don't know about yet. I will look into it.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-03 21:43:36 UTC
Permalink
thsurrel updated this revision to Diff 42818.
thsurrel added a comment.


Fix: filter out all remote types to have the additionnal 'Properties'

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15929?vs=42815&id=42818

BRANCH
arc_properties (branched from master)

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

AFFECTED FILES
src/panels/places/placesitemmodel.cpp
src/panels/places/placesitemmodel.h
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg
Cc: ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Christoph Feck
2018-10-03 22:09:07 UTC
Permalink
cfeck added a comment.


Should 'Properties' be last item in the menu?

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Nathaniel Graham
2018-10-03 22:31:25 UTC
Permalink
ngraham added a comment.


FWIW, I've filed a task to track the places panel code unification work: T9795: Use Places Panel code from KIO instead of private implementation <https://phabricator.kde.org/T9795>

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Christoph Feck
2018-10-03 22:34:03 UTC
Permalink
cfeck added a comment.


Actually, I retract previous comment, it should be the last entry in the first section ('Open in ...'), and the separator that is shown above the 'Edit...' entry should also be visible when the 'Edit...' is hidden.

Open in Window
Open in Tab
Properties ...
-------------------
(Edit....)
Hide Entry
Hide Section

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Nathaniel Graham
2018-10-04 02:53:23 UTC
Permalink
ngraham added a comment.


Works great! We should figure out if this should be here or in KIO though.

INLINE COMMENTS
placesitemmodel.h:52
- explicit PlacesItemModel(QObject* parent = nullptr);
+ explicit PlacesItemModel(QWidget* parent = nullptr);
~PlacesItemModel() override;
Is this necessary?

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-04 07:24:09 UTC
Permalink
thsurrel added a comment.
Post by Christoph Feck
Actually, I retract previous comment, it should be the last entry in the first section ('Open in ...'), and the separator that is shown above the 'Edit...' entry should also be visible when the 'Edit...' is hidden.
Does this mean that we should also add a separator between Properties and the 'hiding' entries in the Device context menu ?

Unmount
-------------------
Open in Window
Open in Tab
Properties ...
------------------- <- to be added ?
Hide Entry
Hide Section

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-04 11:53:24 UTC
Permalink
thsurrel updated this revision to Diff 42844.
thsurrel added a comment.


Keep parent reference as a QObject*

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15929?vs=42818&id=42844

BRANCH
arc_properties (branched from master)

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

AFFECTED FILES
src/panels/places/placesitemmodel.cpp
src/panels/places/placesitemmodel.h
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-04 11:53:46 UTC
Permalink
thsurrel marked an inline comment as done.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Nathaniel Graham
2018-10-04 17:16:43 UTC
Permalink
ngraham added a comment.


I'm okay with doing this here for now as long as we get another patch for KIO as well so that we don't lose any features when we move Dolphin to using KIO's own implementation.

The KIO places panel code is uses everywhere //but// dolphin, so it can be tested in a file open/save dialog, Gwenview, etc.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Christoph Feck
2018-10-04 19:46:10 UTC
Permalink
cfeck added a comment.


Yes, I would prefer if the 'Hide' and 'Edit' entries are always preceeded by the separator, separating the items that act on the actual device and the items that act on the places entry.

But it looks like the separator is a completely unrelated change, so I am fine with a separate diff to fix that.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Christoph Feck
2018-10-04 19:49:48 UTC
Permalink
cfeck added a comment.


Maybe I should actually read the code ... I guess what I want is that you move the 'Properties' above the seperator, not after the 'Edit ...' entry.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Elvis Angelaccio
2018-10-04 20:01:04 UTC
Permalink
elvisangelaccio added a comment.


I'm pretty sure this can be implemented in `KFilePlacesModel` without the need to duplicate the feature here in dolphin.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-04 20:08:04 UTC
Permalink
thsurrel added a comment.
Post by Christoph Feck
Maybe I should actually read the code ... I guess what I want is that you move the 'Properties' above the seperator, not after the 'Edit ...' entry.
Just to be sure I understand you, you mean like this (for Places):

Properties ...
Open in Window
Open in Tab
-------------------
(Edit....)
Hide Entry
Hide Section

And for Devices:

Unmount
-------------------
Properties ...
Open in Window
Open in Tab
Hide Entry
Hide Section

Or should we also add a separtor between 'Open's and 'Hide's in the Devices case ?
(and yes, probably better in another patch, since this one is not sure to make its way in anyway)

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Christoph Feck
2018-10-04 20:29:22 UTC
Permalink
cfeck added a comment.


No, the see the first 'screenshot' I added.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-04 20:34:19 UTC
Permalink
thsurrel added a comment.


Sorry, I misplaced the Properties entry indeed.

But then I am coming back to my question (in my first 'screenshot' as well): shouldn't we add a sperator there ?
Sorry, we are going a bit in circle :)

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Christoph Feck
2018-10-05 00:22:07 UTC
Permalink
cfeck added a comment.


There already is a separator (original line 202), but it is (for whatever reason) not always added. You can remove the `if` for the separator either in this commit, or in a separate patch, because it is unrelated to the addition of the `Properties` item.

What I am trying to get fixed before you commit this is the placement of the new `Properties` item. Order should be:

Open in Window
Open in Tab
(Properties)
--------
(Edit...)
(Remove)
Hide
Hide Section

If I understand your patch, currently the order would be:

Open in Window
Open in Tab
--------
(Edit...)
(Properties)
(Remove)
Hide
Hide Section

(Items in parens are not always visible)

I hope this clarifies the confusion I added by my previous comments.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-05 07:18:08 UTC
Permalink
thsurrel added a comment.


Very clear, there is indeed something to correct in my patch and I will.
But my question was on a different point. The _Devices_ context menu is slightly different than the _Places_ one. I was wondering if we should add a separator between the 'Open in ..." and the 'Hide ...' entries there.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Anthony Fieroni
2018-10-05 07:38:46 UTC
Permalink
anthonyfieroni added inline comments.

INLINE COMMENTS
placesitemmodel.cpp:65
+ m_sourceModel(DolphinPlacesModelSingleton::instance().placesModel()),
+ m_parent(parent)
{
Not needed
placesitemmodel.cpp:313
+
+ if (drive->driveType() != Solid::StorageDrive::HardDisk) {
+ return nullptr;
Check for device against nullptr again.
placesitemmodel.cpp:338
+ if (item->udi().isEmpty()) {
+ dialog = new KPropertiesDialog(item->url(), (QWidget *)m_parent);
+ } else {
Don't use parent when it is not needed, Qt::WA_DeleteOnClose is fair enough.
placesitemmodel.cpp:342
+ Solid::StorageAccess* access = device.as<Solid::StorageAccess>();
+ dialog = new KPropertiesDialog(QUrl("file://" + access->filePath()), (QWidget *)m_parent);
+ }
Same as above.
placesitemmodel.h:237
+
+ QObject* m_parent;
};
Not needed.
placespanel.cpp:176
const bool isTrash = (item->url().scheme() == QLatin1String("trash"));
+
if (isDevice) {
Remove unrelated changes.
placespanel.cpp:217
+ }
+
+ }
Remove white-space line

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-05 08:34:09 UTC
Permalink
thsurrel updated this revision to Diff 42912.
thsurrel added a comment.


Fixes per anthonyfieroni comments. Thanks for the review
Moved the new 'Properties' entry above 'Edit'
Simplified filtering by comparing scheme to "file"

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15929?vs=42844&id=42912

BRANCH
arc_properties (branched from master)

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

AFFECTED FILES
src/panels/places/placesitemmodel.cpp
src/panels/places/placesitemmodel.h
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-05 08:34:59 UTC
Permalink
thsurrel marked 7 inline comments as done.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-06 19:49:10 UTC
Permalink
thsurrel updated this revision to Diff 42983.
thsurrel added a comment.


Use isLocalFile

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15929?vs=42912&id=42983

BRANCH
arc_properties (branched from master)

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

AFFECTED FILES
src/panels/places/placesitemmodel.cpp
src/panels/places/placesitemmodel.h
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Nathaniel Graham
2018-10-06 22:45:06 UTC
Permalink
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.
Post by Elvis Angelaccio
I'm pretty sure this can be implemented in `KFilePlacesModel` without the need to duplicate the feature here in dolphin.
Looks like no, unfortunately. D15973 <https://phabricator.kde.org/D15973> does not automatically result in Dolphin getting the feature.

REPOSITORY
R318 Dolphin

BRANCH
arc_properties (branched from master)

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

To: thsurrel, #dolphin, #plasma, #vdg, ngraham
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Elvis Angelaccio
2018-10-07 08:59:17 UTC
Permalink
elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS
placesitemmodel.cpp:302
+ if (item) {
+ KPropertiesDialog* dialog = new KPropertiesDialog(item->url());
+ dialog->setAttribute(Qt::WA_DeleteOnClose);
The dialog needs a parent, otherwise the window stack will be wrong.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg, ngraham, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-07 13:19:26 UTC
Permalink
thsurrel added inline comments.

INLINE COMMENTS
elvisangelaccio wrote in placesitemmodel.cpp:302
The dialog needs a parent, otherwise the window stack will be wrong.
On the corresponding line in a previous version, anthonyfieroni had commented: "Don't use parent when it is not needed, Qt::WA_DeleteOnClose is fair enough."
Now I am a bit confused when parent is needed or not ...

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg, ngraham, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Elvis Angelaccio
2018-10-07 13:36:23 UTC
Permalink
elvisangelaccio added inline comments.

INLINE COMMENTS
thsurrel wrote in placesitemmodel.cpp:302
On the corresponding line in a previous version, anthonyfieroni had commented: "Don't use parent when it is not needed, Qt::WA_DeleteOnClose is fair enough."
Now I am a bit confused when parent is needed or not ...
Yes, it is needed. In general, any dialog window needs a parent to preserve the window stacking. To check it:

1. Open a properties dialog from the places panel
2. Start the 'Present Windows' kwin effect
3. Select the dolphin window

Without parent, the properties dialog will be behind the dolphin window (which is wrong).

If you repeat the steps using a properties dialog from the dolphin view instead, the dialog will be still visible.

REPOSITORY
R318 Dolphin

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

To: thsurrel, #dolphin, #plasma, #vdg, ngraham, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-07 13:59:05 UTC
Permalink
thsurrel updated this revision to Diff 43039.
thsurrel added a comment.


Pass parent to KPropertiesDialog

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15929?vs=42983&id=43039

BRANCH
arc_properties (branched from master)

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

AFFECTED FILES
src/panels/places/placesitemmodel.cpp
src/panels/places/placesitemmodel.h
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg, ngraham, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-07 18:53:54 UTC
Permalink
thsurrel updated this revision to Diff 43073.
thsurrel added a comment.


dfaure's comment from D15973 <https://phabricator.kde.org/D15973> also applies here: it is better to show the dialog
from the view instead of the model.

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15929?vs=43039&id=43073

BRANCH
arc_properties (branched from master)

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

AFFECTED FILES
src/panels/places/placesitemmodel.h
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg, ngraham, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-11 07:43:46 UTC
Permalink
thsurrel updated this revision to Diff 43374.
thsurrel added a comment.


Rebase

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15929?vs=43073&id=43374

BRANCH
arc_properties (branched from master)

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

AFFECTED FILES
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg, ngraham, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Elvis Angelaccio
2018-10-14 10:17:11 UTC
Permalink
elvisangelaccio accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
R318 Dolphin

BRANCH
arc_properties (branched from master)

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

To: thsurrel, #dolphin, #plasma, #vdg, ngraham, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Thomas Surrel
2018-10-14 10:23:32 UTC
Permalink
This revision was automatically updated to reflect the committed changes.
Closed by commit R318:3f41cd9c0060: Add a &#039;Properties&#039; entry in the Places panel context menu (authored by thsurrel).

CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D15929?vs=43374&id=43580#toc

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D15929?vs=43374&id=43580

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

AFFECTED FILES
src/panels/places/placespanel.cpp

To: thsurrel, #dolphin, #plasma, #vdg, ngraham, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, cfeck, ngraham, abetts, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
Loading...