One thing that strikes me is that a lot of the PlacesItemModel code is just proxying KFilePlacesModel. Another large part of the code seems entirely duplicated. It would make sense to check if we can just use KFilePlacesModel directly in the Places Panel. @renatoo did a larger refactoring of this (D8855), maybe he can offer some insights on this.

In T9795#162803, @nicolasfella wrote:

One thing that strikes me is that a lot of the PlacesItemModel code is just proxying KFilePlacesModel. Another large part of the code seems entirely duplicated. It would make sense to check if we can just use KFilePlacesModel directly in the Places Panel.

I don't think we can. PlacesItemModel is a KStandardItemModel, which is a part of the Dolphin-specific view engine and is not available in KIO.

the Dolphin-specific view engine

Which reminds me of another to-do: T9226: Unify icon/folder views

Here's a quick and dirty proof of concept. Selecting a place works, but I still need to bind:

• middle mouse click
• selecting a pinned place when you navigate in the file view
• selecting the first item on init? selectClosestItem()

And also remove the old "PlaceView/PlaceModel".

diff --git a/src/panels/places/placespanel.cpp b/src/panels/places/placespanel.cpp
index d0dbcce88..d7633d133 100644
--- a/src/panels/places/placespanel.cpp
+++ b/src/panels/places/placespanel.cpp
@@ -39,6 +39,7 @@
 
 #include <KFilePlaceEditDialog>
 #include <KFilePlacesModel>
+#include <KFilePlacesView>
 #include <KIO/DropJob>
 #include <KIO/EmptyTrashJob>
 #include <KIO/Job>
@@ -138,9 +139,15 @@ void PlacesPanel::showEvent(QShowEvent* event)
         KItemListContainer* container = new KItemListContainer(m_controller, this);
         container->setEnabledFrame(false);
 
+        KFilePlacesModel *model = new KFilePlacesModel(this);
+        KFilePlacesView *placesView = new KFilePlacesView(this);
+        placesView->setModel(model);
+        connect(placesView, &KFilePlacesView::urlChanged, this, &PlacesPanel::placeActivated);
+
         QVBoxLayout* layout = new QVBoxLayout(this);
         layout->setContentsMargins(0, 0, 0, 0);
-        layout->addWidget(container);
+        // layout->addWidget(container);
+        layout->addWidget(placesView);
 
         selectClosestItem();
     }

Not sure if I need to wrap the url passed into placeActivated(...) like it does in placespanel.cpp. It seems to parse the search:/documents urls and convert the scheme to baloosearch:/documents.

if (button == Qt::MiddleButton) {
    emit placeMiddleClicked(KFilePlacesModel::convertedUrl(url));
} else {
    emit placeActivated(KFilePlacesModel::convertedUrl(url));
}

• https://github.com/KDE/dolphin/blob/master/src/panels/places/placespanel.cpp#L538
• https://github.com/KDE/kio/blob/master/src/filewidgets/kfileplacesmodel.cpp#L922

Awesome work so far, @Zren! I hope all goes well!

After a deeper look into this:

Summary: There's a number of features that need to be refactored for a final KFilePlacesView patch. A middle mouse click event needs to be implemented. The "open in new tab/window" wouldn't be applicable for the upstream KIO filewidget, so we'd need to monkeypatch the contextmenu (placing it under "Unmount" and above "Properties"). We need to upstream the icon size submenu, or add at least upstream the ability to change the icon size and monkey patch the submenu into the contextmenu. The drop file into pinned folder needs to be refactored to support a folder that needs to be "setup" first.

• middle mouse click
  • KFilePlacesView inherits QListView which only has a left-click signal QAbstractItemView::clicked(const QModelIndex &index). We'll need to create a mouseReleaseEvent(QMouseEvent *) => QModelIndex => PlaceItem => QUrl to implement the middle click DolphinMainWindow::openNewTab.

Note: KFilePlacesView::contextMenuEvent calls QAction *result = menu.exec(event->globalPos()); in a blocking manner it seems, so we won't be able to monkey patch the context menu without modifying the KFilePlacesView to add a "before show" event or overloading the event.

• itemContextMenuRequested
  • "Mount" / "Unmount" / "Safely Remove 'USB Device'"
  • I need to bind "Mount" / "Unmount" error messages.
  • Open in New Window is missing
  • Open in New Tab is missing
  • Properties
  • "Edit" => "Edit Entry 'Trash'"
  • "Hide" => "Hide Entry 'Trash'"
  • "Hide Section" was moved to the contextmenu that appears when you click the "heading".

• viewContextMenuRequested
  • "Icon Sizes" is not a feature of KFilePlacesView, so we need to patch the KIO widget to support different icon sizes at the very least. We'll need to monkey patch the contextmenu if the setting isn't added to the upstream KFilePlacesView.
  • "Show Hidden Places" => "Show All Entries"

• selecting a pinned place (selectClosestItem())
  • the initial url in the addressbar on init
  • when you navigate in the file view

• Drag and Drop
  • Drag folder from file view to pin an item
  • Drag file into pinned folder
    • Needed to set KFilePlacesView::setDropOnPlaceEnabled(true)
    • Need to bind to KFilePlacesView::urlsDropped to PlacesPanel::slotUrlsDropped
    • Unfortunately it's not as easy as that, as Dolphin checks if the dropped folder needs storageSetupNeeded, so I'll need to refactor the drop code.
  • Not sure what the existing behavior is for "drag a pinned item" onto
    • the tab bar
    • the file view

• The "Add to places" from a file view folder contextmenu updates the places panel.

I've personally abandoned attempts to merge KIO's Places view in Dolphin for now. Upstreaming those features to KIO is good code practice in the long term though. I've copied the existing KFilePlacesView from KIO and embeded them in this dolphin branch for easy development:

https://github.com/Zren/dolphin/compare/af32a92...kioplacesfork

The main feature that sets KIO apart from Dolphin is the disk capacity bars for the devices. I plan on submitting a patch that basically draws 2 rectangles underneath the text in PlacesItemWidget.