Differential D10804 Diff 28347 src/dolphinviewcontainer.cpp

Changeset View

Standalone View

View Options

src/dolphinviewcontainer.cpp

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

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

markg: Trash::instance().isEmpty() -> Trash::isEmpty()
97​ m_emptyTrashButton->hide();
98​
88 m_searchBox = new DolphinSearchBox(this); 99​ m_searchBox = new DolphinSearchBox(this);
89 m_searchBox->hide(); 100​ m_searchBox->hide();
90 connect(m_searchBox, &DolphinSearchBox::activated, this, &DolphinViewContainer::activate); 101​ connect(m_searchBox, &DolphinSearchBox::activated, this, &DolphinViewContainer::activate);
91 connect(m_searchBox, &DolphinSearchBox::closeRequest, this, &DolphinViewContainer::closeSearchBox); 102​ connect(m_searchBox, &DolphinSearchBox::closeRequest, this, &DolphinViewContainer::closeSearchBox);
92 connect(m_searchBox, &DolphinSearchBox::searchRequest, this, &DolphinViewContainer::startSearching); 103​ connect(m_searchBox, &DolphinSearchBox::searchRequest, this, &DolphinViewContainer::startSearching);
93 connect(m_searchBox, &DolphinSearchBox::returnPressed, this, &DolphinViewContainer::requestFocus); 104​ connect(m_searchBox, &DolphinSearchBox::returnPressed, this, &DolphinViewContainer::requestFocus);
94 105​
95 m_messageWidget = new KMessageWidget(this); 106​ 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) 155​#if KIO_VERSION >= QT_VERSION_CHECK(5, 37, 0)
145 m_view->dropUrls(destination, event, m_urlNavigator->dropWidget()); 156​ m_view->dropUrls(destination, event, m_urlNavigator->dropWidget());
146#else 157​#else
147 // TODO: remove as soon as we can hard-depend of KF5 >= 5.37 158​ // TODO: remove as soon as we can hard-depend of KF5 >= 5.37
148 m_view->dropUrls(destination, event, m_view); 159​ m_view->dropUrls(destination, event, m_view);
149#endif 160​#endif
150 }); 161​ });
151 162​
163​ connect(m_view, &DolphinView::directoryLoadingCompleted, [this](){
markgUnsubmitted
Done

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

markg: add "this" before the lambda (context thingy).
164​ if (m_view->url().scheme() == QLatin1String("trash")) {
165​ m_emptyTrashButton->show();
166​ } else {
167​ m_emptyTrashButton->hide();
168​ }
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"))` ?
169​ });
170​
152 // Initialize status bar 171​ // Initialize status bar
153 m_statusBar = new DolphinStatusBar(this); 172​ m_statusBar = new DolphinStatusBar(this);
154 m_statusBar->setUrl(m_view->url()); 173​ m_statusBar->setUrl(m_view->url());
155 m_statusBar->setZoomLevel(m_view->zoomLevel()); 174​ m_statusBar->setZoomLevel(m_view->zoomLevel());
156 connect(m_view, &DolphinView::urlChanged, 175​ connect(m_view, &DolphinView::urlChanged,
157 m_statusBar, &DolphinStatusBar::setUrl); 176​ m_statusBar, &DolphinStatusBar::setUrl);
158 connect(m_view, &DolphinView::zoomLevelChanged, 177​ connect(m_view, &DolphinView::zoomLevelChanged,
159 m_statusBar, &DolphinStatusBar::setZoomLevel); 178​ m_statusBar, &DolphinStatusBar::setZoomLevel);
Show All 21 Lines
181 connect(m_filterBar, &FilterBar::filterChanged, 200​ connect(m_filterBar, &FilterBar::filterChanged,
182 this, &DolphinViewContainer::setNameFilter); 201​ this, &DolphinViewContainer::setNameFilter);
183 connect(m_filterBar, &FilterBar::closeRequest, 202​ connect(m_filterBar, &FilterBar::closeRequest,
184 this, &DolphinViewContainer::closeFilterBar); 203​ this, &DolphinViewContainer::closeFilterBar);
185 connect(m_filterBar, &FilterBar::focusViewRequest, 204​ connect(m_filterBar, &FilterBar::focusViewRequest,
186 this, &DolphinViewContainer::requestFocus); 205​ this, &DolphinViewContainer::requestFocus);
187 connect(m_view, &DolphinView::urlChanged, 206​ connect(m_view, &DolphinView::urlChanged,
188 m_filterBar, &FilterBar::slotUrlChanged); 207​ m_filterBar, &FilterBar::slotUrlChanged);
189 208​
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…
190 m_topLayout->addWidget(m_urlNavigator); 209​ navigatorLayout->addWidget(m_urlNavigator);
210​ navigatorLayout->addWidget(m_emptyTrashButton);
211​
212​ m_topLayout->addWidget(m_navigatorWidget);
191 m_topLayout->addWidget(m_searchBox); 213​ m_topLayout->addWidget(m_searchBox);
192 m_topLayout->addWidget(m_messageWidget); 214​ m_topLayout->addWidget(m_messageWidget);
193 m_topLayout->addWidget(m_view); 215​ m_topLayout->addWidget(m_view);
194 m_topLayout->addWidget(m_filterBar); 216​ m_topLayout->addWidget(m_filterBar);
195 m_topLayout->addWidget(m_statusBar); 217​ m_topLayout->addWidget(m_statusBar);
196 218​
197 setSearchModeEnabled(isSearchUrl(url)); 219​ setSearchModeEnabled(isSearchUrl(url));
198 220​
▲ Show 20 Lines • Show All 135 Lines • ▼ Show 20 Line(s)355 if (enabled == isSearchModeEnabled()) {
334 if (enabled && !m_searchBox->hasFocus()) { 356​ if (enabled && !m_searchBox->hasFocus()) {
335 m_searchBox->setFocus(); 357​ m_searchBox->setFocus();
336 m_searchBox->selectAll(); 358​ m_searchBox->selectAll();
337 } 359​ }
338 return; 360​ return;
339 } 361​ }
340 362​
341 m_searchBox->setVisible(enabled); 363​ m_searchBox->setVisible(enabled);
342 m_urlNavigator->setVisible(!enabled); 364​ m_navigatorWidget->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 365​
344 if (enabled) { 366​ if (enabled) {
345 const QUrl& locationUrl = m_urlNavigator->locationUrl(); 367​ const QUrl& locationUrl = m_urlNavigator->locationUrl();
346 m_searchBox->fromSearchUrl(locationUrl); 368​ m_searchBox->fromSearchUrl(locationUrl);
347 } else { 369​ } else {
348 m_view->setViewPropertiesContext(QString()); 370​ m_view->setViewPropertiesContext(QString());
349 371​
350 // Restore the URL for the URL navigator. If Dolphin has been 372​ // Restore the URL for the URL navigator. If Dolphin has been
▲ Show 20 Lines • Show All 352 Lines • Show Last 20 Lines