Differential D10804 Diff 28173 src/dolphinviewcontainer.cpp

Changeset View

Standalone View

View Options

src/dolphinviewcontainer.cpp

Show All 19 Lines
20#include "dolphinviewcontainer.h" 20​#include "dolphinviewcontainer.h"
21#include <KProtocolManager> 21​#include <KProtocolManager>
22 22​
23#include <QDropEvent> 23​#include <QDropEvent>
24#include <QTimer> 24​#include <QTimer>
25#include <QMimeData> 25​#include <QMimeData>
26#include <QVBoxLayout> 26​#include <QVBoxLayout>
27#include <QLoggingCategory> 27​#include <QLoggingCategory>
28​#include <QUrl>
29​#include <QWidget>
28 30​
29#include <KFileItemActions> 31​#include <KFileItemActions>
30#include <KFilePlacesModel> 32​#include <KFilePlacesModel>
31#include <KLocalizedString> 33​#include <KLocalizedString>
32#include <KIO/PreviewJob> 34​#include <KIO/PreviewJob>
33#include <kio_version.h> 35​#include <kio_version.h>
34#include <KMessageWidget> 36​#include <KMessageWidget>
35#include <KShell> 37​#include <KShell>
36#include <QUrl> ​
37#include <KUrlComboBox> 38​#include <KUrlComboBox>
38#include <KUrlNavigator> 39​#include <KUrlNavigator>
39#include <KRun> 40​#include <KRun>
40 41​
41#ifdef KActivities_FOUND 42​#ifdef KActivities_FOUND
42#endif 43​#endif
43 44​
44#include "global.h" 45​#include "global.h"
45#include "dolphindebug.h" 46​#include "dolphindebug.h"
46#include "dolphin_generalsettings.h" 47​#include "dolphin_generalsettings.h"
47#include "filterbar/filterbar.h" 48​#include "filterbar/filterbar.h"
48#include "search/dolphinsearchbox.h" 49​#include "search/dolphinsearchbox.h"
49#include "statusbar/dolphinstatusbar.h" 50​#include "statusbar/dolphinstatusbar.h"
50#include "views/viewmodecontroller.h" 51​#include "views/viewmodecontroller.h"
51#include "views/viewproperties.h" 52​#include "views/viewproperties.h"
53​#include "trash/dolphintrash.h"
54​#include "trash/dolphintrash.h"
52 55​
53DolphinViewContainer::DolphinViewContainer(const QUrl& url, QWidget* parent) : 56​DolphinViewContainer::DolphinViewContainer(const QUrl& url, QWidget* parent) :
54 QWidget(parent), 57​ QWidget(parent),
55 m_topLayout(nullptr), 58​ m_topLayout(nullptr),
56 m_urlNavigator(nullptr), 59​ m_urlNavigator(nullptr),
60​ m_emptyTrashButton(nullptr),
57 m_searchBox(nullptr), 61​ m_searchBox(nullptr),
58 m_messageWidget(nullptr), 62​ m_messageWidget(nullptr),
59 m_view(nullptr), 63​ m_view(nullptr),
60 m_filterBar(nullptr), 64​ m_filterBar(nullptr),
61 m_statusBar(nullptr), 65​ m_statusBar(nullptr),
62 m_statusBarTimer(nullptr), 66​ m_statusBarTimer(nullptr),
63 m_statusBarTimestamp(), 67​ m_statusBarTimestamp(),
64 m_autoGrabFocus(true) 68​ m_autoGrabFocus(true)
65#ifdef KActivities_FOUND 69​#ifdef KActivities_FOUND
66 , m_activityResourceInstance(0) 70​ , m_activityResourceInstance(0)
67#endif 71​#endif
68{ 72​{
69 hide(); 73​ hide();
70 74​
71 m_topLayout = new QVBoxLayout(this); 75​ m_topLayout = new QVBoxLayout(this);
72 m_topLayout->setSpacing(0); 76​ m_topLayout->setSpacing(0);
73 m_topLayout->setMargin(0); 77​ m_topLayout->setMargin(0);
74 78​
79​ QHBoxLayout *navigatorLayout = new QHBoxLayout(this);
80​ navigatorLayout->setMargin(0);
81​
75 m_urlNavigator = new KUrlNavigator(new KFilePlacesModel(this), url, this); 82​ m_urlNavigator = new KUrlNavigator(new KFilePlacesModel(this), url, this);
76 connect(m_urlNavigator, &KUrlNavigator::activated, 83​ connect(m_urlNavigator, &KUrlNavigator::activated,
77 this, &DolphinViewContainer::activate); 84​ this, &DolphinViewContainer::activate);
78 connect(m_urlNavigator->editor(), &KUrlComboBox::completionModeChanged, 85​ connect(m_urlNavigator->editor(), &KUrlComboBox::completionModeChanged,
79 this, &DolphinViewContainer::saveUrlCompletionMode); 86​ this, &DolphinViewContainer::saveUrlCompletionMode);
80 87​
81 const GeneralSettings* settings = GeneralSettings::self(); 88​ const GeneralSettings* settings = GeneralSettings::self();
82 m_urlNavigator->setUrlEditable(settings->editableUrl()); 89​ m_urlNavigator->setUrlEditable(settings->editableUrl());
83 m_urlNavigator->setShowFullPath(settings->showFullPath()); 90​ m_urlNavigator->setShowFullPath(settings->showFullPath());
84 m_urlNavigator->setHomeUrl(Dolphin::homeUrl()); 91​ m_urlNavigator->setHomeUrl(Dolphin::homeUrl());
85 KUrlComboBox* editor = m_urlNavigator->editor(); 92​ KUrlComboBox* editor = m_urlNavigator->editor();
86 editor->setCompletionMode(KCompletion::CompletionMode(settings->urlCompletionMode())); 93​ editor->setCompletionMode(KCompletion::CompletionMode(settings->urlCompletionMode()));
87 94​
95​ m_emptyTrashButton = new QPushButton(QIcon::fromTheme(QStringLiteral("user-trash-symbolic")), "&Empty Trash", this);
rkflxUnsubmitted
Done

Won't do a full code review for Dolphin related patches, but this should just use the same icon as everywhere in Dolphin: user-trash.

rkflx: Won't do a full code review for Dolphin related patches, but this should just use the same icon…
ngrahamUnsubmitted
Done

yeah, listen to Henrik here. This was my bad; we have since determined that we should not be using the -symbolic icons. See D10770#213782

ngraham: yeah, listen to Henrik here. This was my bad; we have since determined that we should //not//…
rkflxUnsubmitted
Done

…because only few icon sets contain this icon. Nevertheless, the HiDPI problem still stands. Perhaps as a workaround we can replicate D10843, i.e. use another icon (providing the same line weight, which -symbolic lacks) and add a fallback.

However, better to use user-trash here, and do the workaround in a follow-up patch (to keep git log clean and change it everywhere at once).

rkflx: …because only few icon sets contain this icon. Nevertheless, the HiDPI problem still stands.
96​ connect(m_emptyTrashButton, &QPushButton::clicked, [this](){
97​ if (emptyTrash(this))
markgUnsubmitted
Done

Trash::instance().empty(this); -> Trash::empty(this);

markg: Trash::instance().empty(this); -> Trash::empty(this);
markgUnsubmitted
Done

add "this" before the lambda (context thingy).

markg: add "this" before the lambda (context thingy).
98​ m_emptyTrashButton->hide();
99​ });
markgUnsubmitted
Done

Trash::instance().isEmpty() -> Trash::isEmpty()

markg: Trash::instance().isEmpty() -> Trash::isEmpty()
100​ m_emptyTrashButton->hide();
101​
88 m_searchBox = new DolphinSearchBox(this); 102​ m_searchBox = new DolphinSearchBox(this);
89 m_searchBox->hide(); 103​ m_searchBox->hide();
90 connect(m_searchBox, &DolphinSearchBox::activated, this, &DolphinViewContainer::activate); 104​ connect(m_searchBox, &DolphinSearchBox::activated, this, &DolphinViewContainer::activate);
91 connect(m_searchBox, &DolphinSearchBox::closeRequest, this, &DolphinViewContainer::closeSearchBox); 105​ connect(m_searchBox, &DolphinSearchBox::closeRequest, this, &DolphinViewContainer::closeSearchBox);
92 connect(m_searchBox, &DolphinSearchBox::searchRequest, this, &DolphinViewContainer::startSearching); 106​ connect(m_searchBox, &DolphinSearchBox::searchRequest, this, &DolphinViewContainer::startSearching);
93 connect(m_searchBox, &DolphinSearchBox::returnPressed, this, &DolphinViewContainer::requestFocus); 107​ connect(m_searchBox, &DolphinSearchBox::returnPressed, this, &DolphinViewContainer::requestFocus);
94 108​
95 m_messageWidget = new KMessageWidget(this); 109​ m_messageWidget = new KMessageWidget(this);
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Line(s)
144#if KIO_VERSION >= QT_VERSION_CHECK(5, 37, 0) 158​#if KIO_VERSION >= QT_VERSION_CHECK(5, 37, 0)
145 m_view->dropUrls(destination, event, m_urlNavigator->dropWidget()); 159​ m_view->dropUrls(destination, event, m_urlNavigator->dropWidget());
146#else 160​#else
147 // TODO: remove as soon as we can hard-depend of KF5 >= 5.37 161​ // TODO: remove as soon as we can hard-depend of KF5 >= 5.37
148 m_view->dropUrls(destination, event, m_view); 162​ m_view->dropUrls(destination, event, m_view);
149#endif 163​#endif
150 }); 164​ });
151 165​
152 // Initialize status bar 166​ // Initialize status bar
markgUnsubmitted
Done

add "this" before the lambda (context thingy).

markg: add "this" before the lambda (context thingy).
153 m_statusBar = new DolphinStatusBar(this); 167​ m_statusBar = new DolphinStatusBar(this);
154 m_statusBar->setUrl(m_view->url()); 168​ m_statusBar->setUrl(m_view->url());
155 m_statusBar->setZoomLevel(m_view->zoomLevel()); 169​ m_statusBar->setZoomLevel(m_view->zoomLevel());
156 connect(m_view, &DolphinView::urlChanged, 170​ connect(m_view, &DolphinView::urlChanged,
157 m_statusBar, &DolphinStatusBar::setUrl); 171​ m_statusBar, &DolphinStatusBar::setUrl);
elvisangelaccioUnsubmitted
Done

How about m_emptyTrashButton->setVisible(m_view->url().scheme() == QLatin1String("trash")) ?

elvisangelaccio: How about `m_emptyTrashButton->setVisible(m_view->url().scheme() == QLatin1String("trash"))` ?
158 connect(m_view, &DolphinView::zoomLevelChanged, 172​ connect(m_view, &DolphinView::zoomLevelChanged,
159 m_statusBar, &DolphinStatusBar::setZoomLevel); 173​ m_statusBar, &DolphinStatusBar::setZoomLevel);
160 connect(m_view, &DolphinView::infoMessage, 174​ connect(m_view, &DolphinView::infoMessage,
161 m_statusBar, &DolphinStatusBar::setText); 175​ m_statusBar, &DolphinStatusBar::setText);
162 connect(m_view, &DolphinView::operationCompletedMessage, 176​ connect(m_view, &DolphinView::operationCompletedMessage,
163 m_statusBar, &DolphinStatusBar::setText); 177​ m_statusBar, &DolphinStatusBar::setText);
164 connect(m_statusBar, &DolphinStatusBar::stopPressed, 178​ connect(m_statusBar, &DolphinStatusBar::stopPressed,
165 this, &DolphinViewContainer::stopDirectoryLoading); 179​ this, &DolphinViewContainer::stopDirectoryLoading);
Show All 15 Lines
181 connect(m_filterBar, &FilterBar::filterChanged, 195​ connect(m_filterBar, &FilterBar::filterChanged,
182 this, &DolphinViewContainer::setNameFilter); 196​ this, &DolphinViewContainer::setNameFilter);
183 connect(m_filterBar, &FilterBar::closeRequest, 197​ connect(m_filterBar, &FilterBar::closeRequest,
184 this, &DolphinViewContainer::closeFilterBar); 198​ this, &DolphinViewContainer::closeFilterBar);
185 connect(m_filterBar, &FilterBar::focusViewRequest, 199​ connect(m_filterBar, &FilterBar::focusViewRequest,
186 this, &DolphinViewContainer::requestFocus); 200​ this, &DolphinViewContainer::requestFocus);
187 connect(m_view, &DolphinView::urlChanged, 201​ connect(m_view, &DolphinView::urlChanged,
188 m_filterBar, &FilterBar::slotUrlChanged); 202​ m_filterBar, &FilterBar::slotUrlChanged);
203​ connect(m_view, &DolphinView::urlChanged, [this](const QUrl &url){
ngrahamUnsubmitted
Done

This syntax is a little unusual. Can you create a separate function that can be connected here rather than defining it inline?

ngraham: This syntax is a little unusual. Can you create a separate function that can be connected here…
rominfAuthorUnsubmitted
Done

It's C++ lambda function from C++11. Very handy for small tasks. Pros: don't pollute namespace with additional definitions, it takes less code.

rominf: It's C++ lambda function from C++11. Very handy for small tasks. Pros: don't pollute namespace…
ngrahamUnsubmitted
Done

I'll let the more experienced Dolphin developers weigh in here, but generally it's best to follow the existing coding style as long as there's nothing explicitly wrong with it, rather than being clever and saving code. Code is cheap!

ngraham: I'll let the more experienced Dolphin developers weigh in here, but generally it's best to…
204​ if (url.scheme() == QLatin1String("trash") && !isTrashEmpty())
205​ m_emptyTrashButton->show();
206​ else
207​ m_emptyTrashButton->hide();
208​ });
209​
210​ navigatorLayout->addWidget(m_urlNavigator);
211​ navigatorLayout->addWidget(m_emptyTrashButton);
189 212​
190 m_topLayout->addWidget(m_urlNavigator); 213​ m_topLayout->addLayout(navigatorLayout);
191 m_topLayout->addWidget(m_searchBox); 214​ m_topLayout->addWidget(m_searchBox);
192 m_topLayout->addWidget(m_messageWidget); 215​ m_topLayout->addWidget(m_messageWidget);
193 m_topLayout->addWidget(m_view); 216​ m_topLayout->addWidget(m_view);
194 m_topLayout->addWidget(m_filterBar); 217​ m_topLayout->addWidget(m_filterBar);
195 m_topLayout->addWidget(m_statusBar); 218​ m_topLayout->addWidget(m_statusBar);
196 219​
197 setSearchModeEnabled(isSearchUrl(url)); 220​ setSearchModeEnabled(isSearchUrl(url));
198 221​
▲ Show 20 Lines • Show All 135 Lines • ▼ Show 20 Line(s)356 if (enabled == isSearchModeEnabled()) {
334 if (enabled && !m_searchBox->hasFocus()) { 357​ if (enabled && !m_searchBox->hasFocus()) {
335 m_searchBox->setFocus(); 358​ m_searchBox->setFocus();
336 m_searchBox->selectAll(); 359​ m_searchBox->selectAll();
337 } 360​ }
338 return; 361​ return;
339 } 362​ }
340 363​
341 m_searchBox->setVisible(enabled); 364​ m_searchBox->setVisible(enabled);
342 m_urlNavigator->setVisible(!enabled); 365​ m_urlNavigator->setVisible(!enabled);
markgUnsubmitted
Done

Where did:
m_urlNavigator->setVisible(!enabled);

go to? Is there a reason for that?

markg: Where did: m_urlNavigator->setVisible(!enabled); go to? Is there a reason for that?
rominfAuthorUnsubmitted
Done

Now m_urlNavigator is contained in m_navigatorWidget. m_navigatorWidget is a droped-in replacement to m_urlNavigator. That means all operations with m_urlNavigator use m_navigatorWidget.

rominf: Now `m_urlNavigator` is contained in `m_navigatorWidget`. `m_navigatorWidget` is a droped-in…
343 366​
344 if (enabled) { 367​ if (enabled) {
345 const QUrl& locationUrl = m_urlNavigator->locationUrl(); 368​ const QUrl& locationUrl = m_urlNavigator->locationUrl();
346 m_searchBox->fromSearchUrl(locationUrl); 369​ m_searchBox->fromSearchUrl(locationUrl);
347 } else { 370​ } else {
348 m_view->setViewPropertiesContext(QString()); 371​ m_view->setViewPropertiesContext(QString());
349 372​
350 // Restore the URL for the URL navigator. If Dolphin has been 373​ // Restore the URL for the URL navigator. If Dolphin has been
▲ Show 20 Lines • Show All 352 Lines • Show Last 20 Lines