Differential D13901 Diff 42128 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​ event->accept();
103​ return true;
104​ }
105​ // I want only catch a tap, when using the TapGesture I need a lot more code
106​ if (event->type() == QEvent::TouchEnd) {
107​ event->accept();
108​ QTouchEvent* touchEvent = static_cast<QTouchEvent*>(event);
109​ const QPoint pos = touchEvent->touchPoints().first().pos().toPoint();
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​ const QPoint startPos = touchEvent->touchPoints().first().startPos().toPoint();
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​ if ((pos - startPos).manhattanLength() < Touch::wiggelRoomForTap) {
112​ emit expand(indexAt(pos));
113​ emit activated(indexAt(pos));
114​ }
115​ return true;
116​ }
117​ return QTreeView::viewportEvent(event);
118​ }
95private: 119​private:
96 QRect mDropRect; 120​ QRect mDropRect;
97}; 121​};
98 122​
99 123​
100FolderViewContextManagerItem::FolderViewContextManagerItem(ContextManager* manager) 124​FolderViewContextManagerItem::FolderViewContextManagerItem(ContextManager* manager)
101: AbstractContextManagerItem(manager) 125​: AbstractContextManagerItem(manager)
102{ 126​{
▲ Show 20 Lines • Show All 99 Lines • ▼ Show 20 Line(s)215{
202 226​
203 // This is tricky: QTreeView header has stretchLastSection set to true. 227​ // This is tricky: QTreeView header has stretchLastSection set to true.
204 // In this configuration, the header gets quite wide and cause an 228​ // In this configuration, the header gets quite wide and cause an
205 // horizontal scrollbar to appear. 229​ // horizontal scrollbar to appear.
206 // To avoid this, set stretchLastSection to false and resizeMode to 230​ // To avoid this, set stretchLastSection to false and resizeMode to
207 // Stretch (we still want the column to take the full width of the 231​ // Stretch (we still want the column to take the full width of the
208 // widget). 232​ // widget).
209 mView->header()->setStretchLastSection(false); 233​ mView->header()->setStretchLastSection(false);
210 mView->header()->setSectionResizeMode(QHeaderView::ResizeToContents); 234​ mView->header()->setSectionResizeMode(QHeaderView::ResizeToContents);
211 235​
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.
236​ 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.
237​
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); 238​ 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); 239​ QObject::connect(mView, &QTreeView::activated, this, &FolderViewContextManagerItem::slotActivated);
214 EventWatcher::install(mView, QEvent::Show, this, SLOT(expandToSelectedUrl())); 240​ 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} 241​}
rkflxUnsubmitted
Not Done

Remove extra space before ).

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