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

No reason to change the whitespace here, methinks.

ngraham: No reason to change the whitespace here, methinks.
32// KDE 35​// KDE
33#include <KUrlMimeData> 36​#include <KUrlMimeData>
34 37​
35// Local 38​// Local
36#include <lib/contextmanager.h> 39​#include <lib/contextmanager.h>
37#include <lib/eventwatcher.h> 40​#include <lib/eventwatcher.h>
38#include "sidebar.h" 41​#include "sidebar.h"
39#include "fileoperations.h" 42​#include "fileoperations.h"
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Line(s)87 {
86 const QModelIndex index = indexAt(event->pos()); 89​ const QModelIndex index = indexAt(event->pos());
87 if (!index.isValid()) { 90​ if (!index.isValid()) {
88 qWarning() << "Invalid index!"; 91​ qWarning() << "Invalid index!";
89 return; 92​ return;
90 } 93​ }
91 const QUrl destUrl = static_cast<MODEL_CLASS*>(model())->urlForIndex(index); 94​ const QUrl destUrl = static_cast<MODEL_CLASS*>(model())->urlForIndex(index);
92 FileOperations::showMenuForDroppedUrls(this, urlList, destUrl); 95​ FileOperations::showMenuForDroppedUrls(this, urlList, destUrl);
93 } 96​ }
97​ bool viewportEvent(QEvent *event) Q_DECL_OVERRIDE
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…
98​ {
99​ if (!QGuiApplication::styleHints()->singleClickActivation()) {
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…
100​ if (event->type() == QEvent::TouchBegin) {
101​ event->accept();
102​ return true;
103​ }
104​ if (event->type() == QEvent::TouchEnd) {
105​ event->accept();
106​ QTouchEvent *touchEvent = static_cast<QTouchEvent *>(event);
107​ QPoint pos = touchEvent->touchPoints().at(0).pos().toPoint();
108​ QPoint startPos = touchEvent->touchPoints().at(0).startPos().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)
109​ if ((pos - startPos).manhattanLength()<10) {
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…
110​ emit expand(indexAt(pos));
111​ emit activated(indexAt(pos));
112​ }
113​ return true;
114​ }
115​ }
116​ return QTreeView::viewportEvent(event);
117​ }
94 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)
▲ Show 20 Lines • Show All 100 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);
235​ QScroller* mScroller = QScroller::scroller(mView->viewport());
ngrahamUnsubmitted
Done

Unnecessary comment

ngraham: Unnecessary comment
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…
rkflxUnsubmitted
Not Done

Only actual member variables should get an m prefix.

rkflx: Only actual member variables should get an `m` prefix.
236​ QScrollerProperties scrollerProp = mScroller->scrollerProperties();
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​ scrollerProp.setScrollMetric(QScrollerProperties::HorizontalOvershootPolicy,1);
238​ scrollerProp.setScrollMetric(QScrollerProperties::VerticalOvershootPolicy,1);
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`.
239​ mScroller->setScrollerProperties(scrollerProp);
240​ mScroller->grabGesture(mView->viewport(), QScroller::TouchGesture);
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.
241​ mView->viewport()->setAttribute(Qt::WA_AcceptTouchEvents );
rkflxUnsubmitted
Not Done

Remove extra space before ).

rkflx: Remove extra space before `)`.
211 242​
212 setWidget(mView); 243​ setWidget(mView);
213 QObject::connect(mView, &QTreeView::activated, this, &FolderViewContextManagerItem::slotActivated); 244​ QObject::connect(mView, &QTreeView::activated, this, &FolderViewContextManagerItem::slotActivated);
214 EventWatcher::install(mView, QEvent::Show, this, SLOT(expandToSelectedUrl())); 245​ EventWatcher::install(mView, QEvent::Show, this, SLOT(expandToSelectedUrl()));
215} 246​}
216 247​
217QModelIndex FolderViewContextManagerItem::findClosestIndex(const QModelIndex& parent, const QUrl& wantedUrl) 248​QModelIndex FolderViewContextManagerItem::findClosestIndex(const QModelIndex& parent, const QUrl& wantedUrl)
218{ 249​{
▲ Show 20 Lines • Show All 55 Lines • Show Last 20 Lines