alexde added a comment.
Post by Nathaniel GrahamDolphin currently does not use the KIO code for parts of its places panel, so this change needs to be replicated for the KIO code so that Places panels in the open/save dialogs and Gwenview (etc.) get the change too. Can you submit a patch for KIO too? The code is in `<kio repo>/src/panels/places/placespanel.cpp` Thanks! Once you submit that patch and both are accepted, we can land them both.
Are you sure about the path and file? I could not find it in the kio repository <https://github.com/KDE/kio/tree/master/src>. However, I found another cpp file which looked promising: <kio repo>/src/filewidgets/kfileplacesview.cpp <https://github.com/KDE/kio/blob/master/src/filewidgets/kfileplacesview.cpp>.
There I would propose the following change:
diff --git a/src/filewidgets/kfileplacesview.cpp b/src/filewidgets/kfileplacesview.cpp
index eb41c6ae..b10eef70 100644
--- a/src/filewidgets/kfileplacesview.cpp
+++ b/src/filewidgets/kfileplacesview.cpp
@@ -742,7 +742,7 @@ void KFilePlacesView::contextMenuEvent(QContextMenuEvent *event)
const bool clickOverHeader = delegate->pointIsHeaderArea(event->pos());
if (clickOverHeader) {
const KFilePlacesModel::GroupType type = placesModel->groupType(index);
- hideSection = menu.addAction(i18n("Hide Section"));
+ hideSection = menu.addAction(QIcon::fromTheme(QStringLiteral("hint")), i18n("Hide Section"));
hideSection->setCheckable(true);
hideSection->setChecked(placesModel->isGroupHidden(type));
}
@@ -778,7 +778,7 @@ void KFilePlacesView::contextMenuEvent(QContextMenuEvent *event)
add = menu.addAction(QIcon::fromTheme(QStringLiteral("document-new")), i18n("Add Entry..."));
}
- hide = menu.addAction(i18n("&Hide Entry '%1'", label));
+ hide = menu.addAction(QIcon::fromTheme(QStringLiteral("hint")), i18n("&Hide Entry '%1'", label));
hide->setCheckable(true);
hide->setChecked(placesModel->isHidden(index));
// if a parent is hidden no interaction should be possible with children, show it first to do so
@@ -790,7 +790,7 @@ void KFilePlacesView::contextMenuEvent(QContextMenuEvent *event)
QAction *showAll = nullptr;
if (placesModel->hiddenCount() > 0) {
- showAll = new QAction(i18n("&Show All Entries"), &menu);
+ showAll = new QAction(QIcon::fromTheme(QStringLiteral("visibility")), i18n("&Show All Entries"), &menu);
showAll->setCheckable(true);
showAll->setChecked(d->showAll);
if (mainSeparator == nullptr) {
---------
Furthermore, I found that at some places in the Dolphin source code QStringLiteral is inconsistently used, for instance:
<dolphin repo>/src/panels/places/placespanel.cpp#L211 <https://github.com/KDE/dolphin/blob/master/src/panels/places/placespanel.cpp#L211>
editAction = menu.addAction(QIcon::fromTheme("edit-entry"), i18nc("@item:inmenu", "Edit..."));
and
<dolphin repo>/src/panels/places/placespanel.cpp#L216 <https://github.com/KDE/dolphin/blob/master/src/panels/places/placespanel.cpp#L216>
removeAction = menu.addAction(QIcon::fromTheme(QStringLiteral("edit-delete")), i18nc("@item:inmenu", "Remove"));
Which version is correct / preferred in which cases?
REPOSITORY
R318 Dolphin
REVISION DETAIL
https://phabricator.kde.org/D16019
To: alexde, #vdg, ngraham
Cc: ngraham, acrouthamel, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp