Differential D13901 Diff 37272 app/folderviewcontextmanageritem.cpp

Changeset View

Standalone View

View Options

app/folderviewcontextmanageritem.cpp

Show All 22 Lines
23 23​
24// Qt 24​// Qt
25#include <QDragEnterEvent> 25​#include <QDragEnterEvent>
26#include <QHeaderView> 26​#include <QHeaderView>
27#include <QTreeView> 27​#include <QTreeView>
28#include <QDir> 28​#include <QDir>
29#include <QMimeData> 29​#include <QMimeData>
30#include <QDebug> 30​#include <QDebug>
31​#include <QScroller>
31 32​
ngrahamUnsubmitted
Done

No reason to change the whitespace here, methinks.

ngraham: No reason to change the whitespace here, methinks.
32// KDE 33​// KDE
33#include <KUrlMimeData> 34​#include <KUrlMimeData>
34 35​
35// Local 36​// Local
36#include <lib/contextmanager.h> 37​#include <lib/contextmanager.h>
37#include <lib/eventwatcher.h> 38​#include <lib/eventwatcher.h>
38#include "sidebar.h" 39​#include "sidebar.h"
39#include "fileoperations.h" 40​#include "fileoperations.h"
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Line(s)85 {
86 const QModelIndex index = indexAt(event->pos()); 87​ const QModelIndex index = indexAt(event->pos());
87 if (!index.isValid()) { 88​ if (!index.isValid()) {
88 qWarning() << "Invalid index!"; 89​ qWarning() << "Invalid index!";
89 return; 90​ return;
90 } 91​ }
91 const QUrl destUrl = static_cast<MODEL_CLASS*>(model())->urlForIndex(index); 92​ const QUrl destUrl = static_cast<MODEL_CLASS*>(model())->urlForIndex(index);
92 FileOperations::showMenuForDroppedUrls(this, urlList, destUrl); 93​ FileOperations::showMenuForDroppedUrls(this, urlList, destUrl);
93 } 94​ }
94 95​
rkflxUnsubmitted
Not Done

Q_DECL_OVERRIDE → override

(28754fa2d95e changed this globally)

rkflx: `Q_DECL_OVERRIDE` → `override` (28754fa2d95e changed this globally)
rkflxUnsubmitted
Not Done

QEvent* event is the preferred style for Gwenview.

(I won't add the same comment again, but please change all other places accordingly.)

rkflx: `QEvent* event` is the preferred style for Gwenview. (I won't add the same comment again, but…
95private: 96​private:
96 QRect mDropRect; 97​ QRect mDropRect;
rkflxUnsubmitted
Not Done

Add comment, e.g.

// Even with mouse behaviour set to double-click,
// a single tap should activate a folder

(otherwise it might not be obvious why this was done later on)

rkflx: Add comment, e.g. // Even with mouse behaviour set to double-click, // a single tap should…
97}; 98​};
98 99​
99 100​
100FolderViewContextManagerItem::FolderViewContextManagerItem(ContextManager* manager) 101​FolderViewContextManagerItem::FolderViewContextManagerItem(ContextManager* manager)
101: AbstractContextManagerItem(manager) 102​: AbstractContextManagerItem(manager)
102{ 103​{
103 mModel = nullptr; 104​ mModel = nullptr;
104 105​
105 setupView(); 106​ setupView();
rkflxUnsubmitted
Not Done

Not strictly necessary, but adding const would make it clear that those variables are just a shorthand and are not intended to be changed.

rkflx: Not strictly necessary, but adding `const` would make it clear that those variables are just a…
rkflxUnsubmitted
Not Done

at(0) → first() reads a bit more fluently

(again, see below for more occurrences of this)

rkflx: `at(0)` → `first()` reads a bit more fluently (again, see below for more occurrences of this)
106 107​
rkflxUnsubmitted
Not Done

Add spaces around <

rkflx: Add spaces around `<`
rkflxUnsubmitted
Not Done

I wonder where that magic 10 comes from?

In general this code block reads like you want to catch a regular tap. Would using QTapGesture work too?

rkflx: I wonder where that magic `10` comes from? In general this code block reads like you want to…
107 connect(contextManager(), SIGNAL(currentDirUrlChanged(QUrl)), 108​ connect(contextManager(), SIGNAL(currentDirUrlChanged(QUrl)),
108 SLOT(slotCurrentDirUrlChanged(QUrl))); 109​ SLOT(slotCurrentDirUrlChanged(QUrl)));
109} 110​}
110 111​
111void FolderViewContextManagerItem::slotCurrentDirUrlChanged(const QUrl &url) 112​void FolderViewContextManagerItem::slotCurrentDirUrlChanged(const QUrl &url)
112{ 113​{
113 if (url.isValid() && mUrlToSelect != url) { 114​ if (url.isValid() && mUrlToSelect != url) {
114 mUrlToSelect = url.adjusted(QUrl::StripTrailingSlash | QUrl::NormalizePathSegments); 115​ mUrlToSelect = url.adjusted(QUrl::StripTrailingSlash | QUrl::NormalizePathSegments);
▲ Show 20 Lines • Show All 87 Lines • ▼ Show 20 Line(s)192{
202 203​
203 // This is tricky: QTreeView header has stretchLastSection set to true. 204​ // This is tricky: QTreeView header has stretchLastSection set to true.
204 // In this configuration, the header gets quite wide and cause an 205​ // In this configuration, the header gets quite wide and cause an
205 // horizontal scrollbar to appear. 206​ // horizontal scrollbar to appear.
206 // To avoid this, set stretchLastSection to false and resizeMode to 207​ // To avoid this, set stretchLastSection to false and resizeMode to
207 // Stretch (we still want the column to take the full width of the 208​ // Stretch (we still want the column to take the full width of the
208 // widget). 209​ // widget).
209 mView->header()->setStretchLastSection(false); 210​ mView->header()->setStretchLastSection(false);
210 mView->header()->setSectionResizeMode(QHeaderView::ResizeToContents); 211​ mView->header()->setSectionResizeMode(QHeaderView::ResizeToContents);
211 212​
ngrahamUnsubmitted
Done

Unnecessary comment

ngraham: Unnecessary comment
rkflxUnsubmitted
Not Done

Only actual member variables should get an m prefix.

rkflx: Only actual member variables should get an `m` prefix.
213​ QScroller::grabGesture(mView->viewport(), QScroller::TouchGesture);
rkflxUnsubmitted
Not Done

There should be enough visual and hard disk space for not having to abbreviate Prop, i.e. rename to scrollerProperties.

rkflx: There should be enough visual and hard disk space for not having to abbreviate `Prop`, i.e.
214​
rkflxUnsubmitted
Not Done

Separate these lines with a newline to group the code into logical blocks and thus make it more readable.

rkflx: Separate these lines with a newline to group the code into logical blocks and thus make it more…
212 setWidget(mView); 215​ setWidget(mView);
rkflxUnsubmitted
Not Done

Add space after commas.

rkflx: Add space after commas.
rkflxUnsubmitted
Not Done

Always use enum values by name, e.g. QScrollerProperties::OvershootAlwaysOff instead of 1.

rkflx: Always use `enum` values by name, e.g. `QScrollerProperties::OvershootAlwaysOff` instead of `1`.
213 QObject::connect(mView, &QTreeView::activated, this, &FolderViewContextManagerItem::slotActivated); 216​ QObject::connect(mView, &QTreeView::activated, this, &FolderViewContextManagerItem::slotActivated);
214 EventWatcher::install(mView, QEvent::Show, this, SLOT(expandToSelectedUrl())); 217​ EventWatcher::install(mView, QEvent::Show, this, SLOT(expandToSelectedUrl()));
rkflxUnsubmitted
Not Done

Since TouchGesture is the default value for the second argument, it can be left out.

rkflx: Since `TouchGesture` is the default value for the second argument, it can be left out.
215} 218​}
rkflxUnsubmitted
Not Done

Remove extra space before ).

rkflx: Remove extra space before `)`.
216 219​
217QModelIndex FolderViewContextManagerItem::findClosestIndex(const QModelIndex& parent, const QUrl& wantedUrl) 220​QModelIndex FolderViewContextManagerItem::findClosestIndex(const QModelIndex& parent, const QUrl& wantedUrl)
218{ 221​{
219 Q_ASSERT(mModel); 222​ Q_ASSERT(mModel);
220 QModelIndex index = parent; 223​ QModelIndex index = parent;
221 if (!index.isValid()) { 224​ if (!index.isValid()) {
222 index = findRootIndex(wantedUrl); 225​ index = findRootIndex(wantedUrl);
223 if (!index.isValid()) { 226​ if (!index.isValid()) {
▲ Show 20 Lines • Show All 50 Lines • Show Last 20 Lines