Apply suggestions from code review

qgis · Dec 12, 2024 · f4d991f · f4d991f
1 parent 14ce7ea
commit f4d991f
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 8 deletions.
diff --git a/src/app/layout/qgslayoutdesignerdialog.cpp b/src/app/layout/qgslayoutdesignerdialog.cpp
@@ -613,8 +613,8 @@ QgsLayoutDesignerDialog::QgsLayoutDesignerDialog( QWidget *parent, Qt::WindowFla
 #endif
 
   // Delete selection should be disabled when creating node item
-  connect( mAddNodeItemTool, &QgsLayoutViewTool::activated, this, [ this, backSpace ] { backSpace->setEnabled( false ); mActionDeleteSelection->setEnabled( false ); } );
-  connect( mAddNodeItemTool, &QgsLayoutViewTool::deactivated, this, [ this, backSpace ] { backSpace->setEnabled( true ); mActionDeleteSelection->setEnabled( true ); } );
+  connect( mAddNodeItemTool, &QgsLayoutViewTool::activated, this, [this, backSpace] { backSpace->setEnabled( false ); mActionDeleteSelection->setEnabled( false ); } );
+  connect( mAddNodeItemTool, &QgsLayoutViewTool::deactivated, this, [this, backSpace] { backSpace->setEnabled( true ); mActionDeleteSelection->setEnabled( true ); } );
 
 
   mActionPreviewModeOff->setChecked( true );
@@ -3936,11 +3936,6 @@ void QgsLayoutDesignerDialog::activateNewItemCreationTool( int id, bool nodeBase
     if ( mView )
       mView->setTool( mAddNodeItemTool );
   }
-
-  if ( mLayout )
-  {
-    mLayout->deselectAll();
-  }
 }
 
 void QgsLayoutDesignerDialog::createLayoutPropertiesWidget()

diff --git a/src/gui/layout/qgslayoutviewtooladdnodeitem.cpp b/src/gui/layout/qgslayoutviewtooladdnodeitem.cpp
@@ -55,6 +55,9 @@ void QgsLayoutViewToolAddNodeItem::layoutPressEvent( QgsLayoutViewMouseEvent *ev
       mRubberBand.reset( QgsGui::layoutItemGuiRegistry()->createNodeItemRubberBand( mItemMetadataId, view() ) );
       if ( mRubberBand )
         layout()->addItem( mRubberBand.get() );
+
+      // On first press, deselect the currently selected item, if any
+      view()->deselectAll();
     }
 
     if ( mRubberBand )
@@ -192,8 +195,14 @@ void QgsLayoutViewToolAddNodeItem::moveTemporaryNode( QPointF scenePoint, Qt::Ke
 void QgsLayoutViewToolAddNodeItem::setRubberBandNodes()
 {
   QList<QGraphicsItem *> items = mRubberBand->childItems();
+
+  // Rubber band is not a QGraphicsItem with children, but may be
+  // a custom QGraphicsPolygonItem / QGraphicsPathItem returned by a Python plugin
+  // In this case, directly append it to the list.
   if ( items.isEmpty() )
-    return;
+  {
+    items << mRubberBand.get();
+  }
 
   if ( QGraphicsPolygonItem *polygonItem = dynamic_cast<QGraphicsPolygonItem *>( items[0] ) )
   {