Differential D13901 Diff 54003 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 <QStyleHints>
32​#include <QApplication>
31 33​
ngrahamUnsubmitted
Done

No reason to change the whitespace here, methinks.

ngraham: No reason to change the whitespace here, methinks.
32// KDE 34​// KDE
33#include <KUrlMimeData> 35​#include <KUrlMimeData>
34 36​
35// Local 37​// Local
36#include <lib/contextmanager.h> 38​#include <lib/contextmanager.h>
37#include <lib/eventwatcher.h> 39​#include <lib/eventwatcher.h>
38#include "sidebar.h" 40​#include "sidebar.h"
39#include "fileoperations.h" 41​#include "fileoperations.h"
42​#include <lib/scrollerutils.h>
43​#include "lib/touch/touch_helper.h"
40 44​
41namespace Gwenview 45​namespace Gwenview
42{ 46​{
43 47​
44/** 48​/**
45 * This treeview accepts url drops 49​ * This treeview accepts url drops
46 */ 50​ */
47class UrlDropTreeView : public QTreeView 51​class UrlDropTreeView : public QTreeView
Show All 38 Lines88 {
86 const QModelIndex index = indexAt(event->pos()); 90​ const QModelIndex index = indexAt(event->pos());
87 if (!index.isValid()) { 91​ if (!index.isValid()) {
88 qWarning() << "Invalid index!"; 92​ qWarning() << "Invalid index!";
89 return; 93​ return;
90 } 94​ }
91 const QUrl destUrl = static_cast<MODEL_CLASS*>(model())->urlForIndex(index); 95​ const QUrl destUrl = static_cast<MODEL_CLASS*>(model())->urlForIndex(index);
92 FileOperations::showMenuForDroppedUrls(this, urlList, destUrl); 96​ FileOperations::showMenuForDroppedUrls(this, urlList, destUrl);
93 } 97​ }
94 98​
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…
99​ bool viewportEvent(QEvent* event) override
100​ {
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…
101​ if (event->type() == QEvent::TouchBegin) {
102​ return true;
103​ }
104​ const QPoint pos = Touch_Helper::simpleTapPosition(event);
105​ if (pos != QPoint(-1, -1)) {
106​ expand(indexAt(pos));
107​ emit activated(indexAt(pos));
108​ }
109​
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)
110​ return QTreeView::viewportEvent(event);
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…
111​ }
95private: 112​private:
96 QRect mDropRect; 113​ QRect mDropRect;
97}; 114​};
98 115​
99 116​
100FolderViewContextManagerItem::FolderViewContextManagerItem(ContextManager* manager) 117​FolderViewContextManagerItem::FolderViewContextManagerItem(ContextManager* manager)
101: AbstractContextManagerItem(manager) 118​: AbstractContextManagerItem(manager)
102{ 119​{
▲ Show 20 Lines • Show All 99 Lines • ▼ Show 20 Line(s)208{
202 219​
203 // This is tricky: QTreeView header has stretchLastSection set to true. 220​ // This is tricky: QTreeView header has stretchLastSection set to true.
204 // In this configuration, the header gets quite wide and cause an 221​ // In this configuration, the header gets quite wide and cause an
205 // horizontal scrollbar to appear. 222​ // horizontal scrollbar to appear.
206 // To avoid this, set stretchLastSection to false and resizeMode to 223​ // To avoid this, set stretchLastSection to false and resizeMode to
207 // Stretch (we still want the column to take the full width of the 224​ // Stretch (we still want the column to take the full width of the
208 // widget). 225​ // widget).
209 mView->header()->setStretchLastSection(false); 226​ mView->header()->setStretchLastSection(false);
210 mView->header()->setSectionResizeMode(QHeaderView::ResizeToContents); 227​ mView->header()->setSectionResizeMode(QHeaderView::ResizeToContents);
211 228​
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.
229​ ScrollerUtils::setQScroller(mView->viewport());
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.
230​
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); 231​ 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); 232​ QObject::connect(mView, &QTreeView::activated, this, &FolderViewContextManagerItem::slotActivated);
214 EventWatcher::install(mView, QEvent::Show, this, SLOT(expandToSelectedUrl())); 233​ 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} 234​}
rkflxUnsubmitted
Not Done

Remove extra space before ).

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