Skip to content

Commit

Permalink
Fix invalid merge of non equal item commands
Browse files Browse the repository at this point in the history
  • Loading branch information
nyalldawson committed Oct 6, 2017
1 parent 88a7f02 commit 66b4bdf
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 4 deletions.
13 changes: 9 additions & 4 deletions src/core/layout/qgslayoutitem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,12 @@ void QgsLayoutItem::setVisibility( const bool visible )
return;
}

if ( mLayout && !mBlockUndoCommands )
if ( !shouldBlockUndoCommands() )
mLayout->undoStack()->beginCommand( this, visible ? tr( "Item shown" ) : tr( "Item hidden" ) );

QGraphicsItem::setVisible( visible );

if ( mLayout && !mBlockUndoCommands )
if ( !shouldBlockUndoCommands() )
mLayout->undoStack()->endCommand();

//inform model that visibility has changed
Expand All @@ -136,12 +136,12 @@ void QgsLayoutItem::setLocked( const bool locked )
return;
}

if ( mLayout && !mBlockUndoCommands )
if ( !shouldBlockUndoCommands() )
mLayout->undoStack()->beginCommand( this, locked ? tr( "Item locked" ) : tr( "Item unlocked" ) );

mIsLocked = locked;

if ( mLayout && !mBlockUndoCommands )
if ( !shouldBlockUndoCommands() )
mLayout->undoStack()->endCommand();

//inform model that id data has changed
Expand Down Expand Up @@ -332,6 +332,11 @@ void QgsLayoutItem::setScenePos( const QPointF &destinationPos )
setPos( pos() + ( destinationPos - scenePos() ) );
}

bool QgsLayoutItem::shouldBlockUndoCommands() const
{
return !mLayout || mLayout != scene() || mBlockUndoCommands;
}

double QgsLayoutItem::itemRotation() const
{
return rotation();
Expand Down
1 change: 1 addition & 0 deletions src/core/layout/qgslayoutitem.h
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,7 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
void updateStoredItemPosition();
QPointF itemPositionAtReferencePoint( const ReferencePoint reference, const QSizeF &size ) const;
void setScenePos( const QPointF &destinationPos );
bool shouldBlockUndoCommands() const;

friend class TestQgsLayoutItem;
};
Expand Down
2 changes: 2 additions & 0 deletions src/core/layout/qgslayoutitemundocommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ bool QgsLayoutItemUndoCommand::mergeWith( const QUndoCommand *command )
{
return false;
}
if ( c->itemUuid() != itemUuid() )
return false;

setAfterState( c->afterState() );
return true;
Expand Down
30 changes: 30 additions & 0 deletions tests/src/core/testqgslayoutitem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class TestQgsLayoutItem: public QObject
void readXml();
void writeReadXmlProperties();
void undoRedo();
void multiItemUndo();

private:

Expand Down Expand Up @@ -1452,6 +1453,35 @@ void TestQgsLayoutItem::undoRedo()

}

void TestQgsLayoutItem::multiItemUndo()
{
QgsProject proj;
QgsLayout l( &proj );

QgsLayoutItemRectangularShape *item = new QgsLayoutItemRectangularShape( &l );
l.addLayoutItem( item );
item->attemptMove( QgsLayoutPoint( 10, 10 ) );
QgsLayoutItemRectangularShape *item2 = new QgsLayoutItemRectangularShape( &l );
l.addLayoutItem( item2 );
item2->attemptMove( QgsLayoutPoint( 20, 20 ) );

l.undoStack()->beginCommand( item, tr( "Item moved" ), QgsLayoutItem::UndoIncrementalMove );
item->attemptMove( QgsLayoutPoint( 1, 1 ) );
l.undoStack()->endCommand();

l.undoStack()->beginCommand( item2, tr( "Item moved" ), QgsLayoutItem::UndoIncrementalMove );
item2->attemptMove( QgsLayoutPoint( 21, 21 ) );
l.undoStack()->endCommand();

// undo should only remove item2 move
l.undoStack()->stack()->undo();
QCOMPARE( item2->positionWithUnits(), QgsLayoutPoint( 20, 20 ) );
QCOMPARE( item->positionWithUnits(), QgsLayoutPoint( 1, 1 ) );
l.undoStack()->stack()->undo();
QCOMPARE( item2->positionWithUnits(), QgsLayoutPoint( 20, 20 ) );
QCOMPARE( item->positionWithUnits(), QgsLayoutPoint( 10, 10 ) );
}

QgsLayoutItem *TestQgsLayoutItem::createCopyViaXml( QgsLayout *layout, QgsLayoutItem *original )
{
//save original item to xml
Expand Down

0 comments on commit 66b4bdf

Please sign in to comment.