Added drag and drop functionalities to the map. You can drag items fr… #50
Conversation
|
There is few issues with code:
Easiest way to fix all pre-commit issues is to run pre-commit under root of repo locally - it will fix all issues for you (https://pre-commit.com/):
Sorry, but I very pedantic around coding conventions :) |
lib/include/QGeoView/QGVMap.h
Outdated
| private: | ||
| QScopedPointer<QGVProjection> mProjection; | ||
| QScopedPointer<QGVMapQGView> mQGView; | ||
| QScopedPointer<QGVItem> mRootItem; | ||
| QList<QGVWidget*> mWidgets; | ||
| QSet<QGVItem*> mSelections; | ||
|
|
||
| private slots: |
There was a problem hiding this comment.
please don't use keywords reserved by Qt directly - it can collide with other frameworks. if you need slots please use instead Q_SLOTS.
Since Qt 5.0 is no need to declare methods as slots as any method can be act as slot for signal connection. Only if you need to call method over QMetaObject::invokeMethod it becomes as mandatory.
There was a problem hiding this comment.
Okay, I will remove it.
lib/include/QGeoView/QGVMap.h
Outdated
| private: | ||
| QScopedPointer<QGVProjection> mProjection; | ||
| QScopedPointer<QGVMapQGView> mQGView; | ||
| QScopedPointer<QGVItem> mRootItem; | ||
| QList<QGVWidget*> mWidgets; | ||
| QSet<QGVItem*> mSelections; | ||
|
|
||
| private slots: | ||
| void handleDropDataOnQGVMapQGView(QPointF position, const QMimeData *dropData); |
There was a problem hiding this comment.
A bit more pedantic :) issues - please keep member variables at the end of class declaration - it mean all your declarations would be better to locate above member variables section.
Ideally:
class
{
public:
..
Q_SIGNALS:
..
protected:
..
private:
...
member variables
}
lib/include/QGeoView/QGVMapQGView.h
Outdated
| @@ -102,4 +113,6 @@ class QGV_LIB_DECL QGVMapQGView : public QGraphicsView | |||
| QScopedPointer<QGraphicsScene> mQGScene; | |||
| QScopedPointer<QGVMapRubberBand> mSelectionRect; | |||
| QScopedPointer<QMenu> mContextMenu; | |||
| signals: | |||
lib/src/QGVMapQGView.cpp
Outdated
| { | ||
| if (event->mimeData()->hasFormat("application/x-qabstractitemmodeldatalist")) | ||
| { | ||
| emit dropData(event->position(), event->mimeData()); |
There was a problem hiding this comment.
Hmm I wounder if we can simply propagate QDropEvent to map. As you have ignored event it should be propagated to parent of item (probably QGraphicsView::dropEvent(event) need to be called before return). In this case we can remove need of signal and instead write logic directly in
QGVMap::dropEvent(QDropEvent* event)
It will give more freedom for future development as QGVMap::dropEvent can be overwritten by anybody and more complex handling can be added (new mime types, etc)
There was a problem hiding this comment.
The first thing I tried was to subclass QGVMap to add the accept drop functionality, because I didn't want to change your library. However QGVMap inherits from QWidget. It looks that QWidget doesn't send the drop event to a child QGraphicsView inside of it. I think this is a limitation of QWidget or QGraphicsView, not sure (maybe this is the correct behavior).
Then I realized that I would need to change QGVMapQGView class to achieve the intended behavior.
Well... I didn't restricted the mime types, so there is a plenty of freedom for new implementations.
There was a problem hiding this comment.
I was wrong. I really restricted the type of mime-data on the QGVMapQGView class. Sorry. I just fixed all the remaining errors.
There was a problem hiding this comment.
Well, I did my best to fix clang-forma error on pre-commit. Unfortunately, I could not fix it locally.
…om a QTreeWidget to the map in order to position it on the map. Unfortunately, the anchor parameter was removed from the setGeometry method in QGVIcon and QGVImage. I would like to see it back to the code.