Deduplicate code relating to shifting item positions in layout units

qgis · Dec 6, 2017 · acb956a · acb956a
1 parent d2fcf11
commit acb956a
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 21 deletions.
diff --git a/src/core/layout/qgslayoutitem.cpp b/src/core/layout/qgslayoutitem.cpp
@@ -435,6 +435,21 @@ void QgsLayoutItem::attemptSetSceneRect( const QRectF &rect, bool includesFrame
   emit sizePositionChanged();
 }
 
+void QgsLayoutItem::attemptMoveBy( double deltaX, double deltaY )
+{
+  if ( !mLayout )
+  {
+    moveBy( deltaX, deltaY );
+    return;
+  }
+
+  QgsLayoutPoint itemPos = positionWithUnits();
+  QgsLayoutPoint deltaPos = mLayout->convertFromLayoutUnits( QPointF( deltaX, deltaY ), itemPos.units() );
+  itemPos.setX( itemPos.x() + deltaPos.x() );
+  itemPos.setY( itemPos.y() + deltaPos.y() );
+  attemptMove( itemPos );
+}
+
 int QgsLayoutItem::page() const
 {
   if ( !mLayout )

diff --git a/src/core/layout/qgslayoutitem.h b/src/core/layout/qgslayoutitem.h
@@ -391,6 +391,7 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
      * Note that the final position of the item may not match the specified target position,
      * as data defined item position may override the specified value.
      *
+     * \see attemptMoveBy()
      * \see attemptResize()
      * \see referencePoint()
      * \see positionWithUnits()
@@ -414,6 +415,20 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
      */
     void attemptSetSceneRect( const QRectF &rect, bool includesFrame = false );
 
+    /**
+     * Attempts to shift the item's position by a specified \a deltaX and \a deltaY, in layout
+     * units.
+     *
+     * Note that the final position of the item may not match the specified offsets,
+     * as data defined item position and size may override the specified value.
+     *
+     * \see attemptResize()
+     * \see attemptMove()
+     * \see referencePoint()
+     * \see positionWithUnits()
+     */
+    void attemptMoveBy( double deltaX, double deltaY );
+
     /**
      * Returns the item's current position, including units. The position returned
      * is the position of the item's reference point, which may not necessarily be the top

diff --git a/src/core/layout/qgslayoutitemgroup.cpp b/src/core/layout/qgslayoutitemgroup.cpp
@@ -153,12 +153,7 @@ void QgsLayoutItemGroup::attemptMove( const QgsLayoutPoint &point, bool useRefer
       command->saveBeforeState();
     }
 
-    // need to convert delta from layout units -> item units
-    QgsLayoutPoint itemPos = item->positionWithUnits();
-    QgsLayoutPoint deltaPos = mLayout->convertFromLayoutUnits( QPointF( deltaX, deltaY ), itemPos.units() );
-    itemPos.setX( itemPos.x() + deltaPos.x() );
-    itemPos.setY( itemPos.y() + deltaPos.y() );
-    item->attemptMove( itemPos, true, includesFrame );
+    item->attemptMoveBy( deltaX, deltaY );
 
     if ( command )
     {

diff --git a/src/gui/layout/qgslayoutmousehandles.cpp b/src/gui/layout/qgslayoutmousehandles.cpp
@@ -608,12 +608,7 @@ void QgsLayoutMouseHandles::mouseReleaseEvent( QGraphicsSceneMouseEvent *event )
       std::unique_ptr< QgsAbstractLayoutUndoCommand > command( item->createCommand( QString(), 0 ) );
       command->saveBeforeState();
 
-      // need to convert delta from layout units -> item units
-      QgsLayoutPoint itemPos = item->positionWithUnits();
-      QgsLayoutPoint deltaPos = mLayout->convertFromLayoutUnits( QPointF( deltaX, deltaY ), itemPos.units() );
-      itemPos.setX( itemPos.x() + deltaPos.x() );
-      itemPos.setY( itemPos.y() + deltaPos.y() );
-      item->attemptMove( itemPos );
+      item->attemptMoveBy( deltaX, deltaY );
 
       command->saveAfterState();
       mLayout->undoStack()->push( command.release() );

diff --git a/src/gui/layout/qgslayoutview.cpp b/src/gui/layout/qgslayoutview.cpp
@@ -926,20 +926,12 @@ void QgsLayoutView::keyPressEvent( QKeyEvent *event )
     const QList<QgsLayoutItem *> layoutItemList = l->selectedLayoutItems();
 
     QPointF delta = deltaForKeyEvent( event );
-    auto moveItem = [ l, delta ]( QgsLayoutItem * item )
-    {
-      QgsLayoutPoint itemPos = item->positionWithUnits();
-      QgsLayoutPoint deltaPos = l->convertFromLayoutUnits( delta, itemPos.units() );
-      itemPos.setX( itemPos.x() + deltaPos.x() );
-      itemPos.setY( itemPos.y() + deltaPos.y() );
-      item->attemptMove( itemPos );
-    };
 
     l->undoStack()->beginMacro( tr( "Move Item" ) );
     for ( QgsLayoutItem *item : layoutItemList )
     {
       l->undoStack()->beginCommand( item, tr( "Move Item" ), QgsLayoutItem::UndoIncrementalMove );
-      moveItem( item );
+      item->attemptMoveBy( delta.x(), delta.y() );
       l->undoStack()->endCommand();
     }
     l->undoStack()->endMacro();

diff --git a/tests/src/core/testqgslayoutitem.cpp b/tests/src/core/testqgslayoutitem.cpp
@@ -1199,6 +1199,21 @@ void TestQgsLayoutItem::move()
   QCOMPARE( item->scenePos().x(), 10.0 );
   QCOMPARE( item->scenePos().y(), 12.0 );
 
+  //moveBy
+  item.reset( new TestItem( &l ) );
+  item->attemptMove( QgsLayoutPoint( 5, 9, QgsUnitTypes::LayoutCentimeters ) );
+  item->attemptResize( QgsLayoutSize( 4, 6 ) );
+  QCOMPARE( item->positionWithUnits().x(), 5.0 );
+  QCOMPARE( item->positionWithUnits().y(), 9.0 );
+  QCOMPARE( item->scenePos().x(), 50.0 );
+  QCOMPARE( item->scenePos().y(), 90.0 );
+  item->attemptMoveBy( 5, -6 );
+  QCOMPARE( item->positionWithUnits().x(), 5.5 );
+  QCOMPARE( item->positionWithUnits().y(), 8.4 );
+  QCOMPARE( item->positionWithUnits().units(), QgsUnitTypes::LayoutCentimeters );
+  QCOMPARE( item->scenePos().x(), 55.0 );
+  QCOMPARE( item->scenePos().y(), 84.0 );
+
   //item with frame
   item.reset( new TestItem( &l ) );
   item->attemptResize( QgsLayoutSize( 2, 4 ) );