Prevent attribute form editor widgets from inherting too large

horizontal minimum size

This bug causes editor widgets to inherit a very large minimum width
from the search widget wrapper, regardless of what mode the attribute
form is shown in. The outcome is that when adding/editing features
all widgets have a large minimum width, causing very wide forms
in multi-column setups with a lot of unnecessary edit widget width.

Fix sponsored by NIWA

python/gui/auto_generated/qgsattributeformwidget.sip.in

@@ -37,6 +37,8 @@ Consists of the widget which is visible in edit mode as well as the widget visib
 A new form widget for the wrapper ``widget`` on ``form``.
 %End
 
+    ~QgsAttributeFormWidget();
+
     virtual void createSearchWidgetWrappers() = 0;
 %Docstring
 Creates the search widget wrappers for the widget used when the form is in
@@ -139,6 +141,13 @@ this search widgte or defines the comparison operator to use.
 
 
 
+    void setVisiblePageForMode( QgsAttributeFormWidget::Mode mode );
+%Docstring
+Sets the visible page in the widget to the page matching the specified ``mode``.
+
+.. versionadded:: 3.32
+%End
+
 };
 
 /************************************************************************

src/gui/qgsattributeformeditorwidget.cpp

@@ -16,14 +16,12 @@
 #include "qgsattributeformeditorwidget.h"
 #include "qgsattributeform.h"
 #include "qgsmultiedittoolbutton.h"
-#include "qgssearchwidgettoolbutton.h"
 #include "qgseditorwidgetwrapper.h"
 #include "qgssearchwidgetwrapper.h"
 #include "qgsattributeeditorcontext.h"
 #include "qgseditorwidgetregistry.h"
 #include "qgsaggregatetoolbutton.h"
 #include "qgsgui.h"
-#include "qgsvectorlayerjoinbuffer.h"
 #include "qgsvectorlayerutils.h"
 
 #include <QLayout>
@@ -286,27 +284,26 @@ void QgsAttributeFormEditorWidget::updateWidgets()
     editPage()->layout()->addWidget( mMultiEditButton );
   }
 
+  setVisiblePageForMode( mode() );
+
   switch ( mode() )
   {
     case DefaultMode:
     case MultiEditMode:
     {
-      stack()->setCurrentWidget( editPage() );
       editPage()->layout()->addWidget( mConstraintResultLabel );
       break;
     }
 
     case AggregateSearchMode:
     {
       mAggregateButton->setVisible( true );
-      stack()->setCurrentWidget( searchPage() );
       break;
     }
 
     case SearchMode:
     {
       mAggregateButton->setVisible( false );
-      stack()->setCurrentWidget( searchPage() );
       break;
     }
   }

src/gui/qgsattributeformeditorwidget.h

@@ -151,7 +151,7 @@ class GUI_EXPORT QgsAttributeFormEditorWidget : public QgsAttributeFormWidget
     bool mIsMixed;
     bool mIsChanged;
 
-    void updateWidgets() override;
+    void updateWidgets() final;
 
     friend class TestQgsAttributeForm;
 };

src/gui/qgsattributeformwidget.cpp

@@ -45,10 +45,17 @@ QgsAttributeFormWidget::QgsAttributeFormWidget( QgsWidgetWrapper *widget, QgsAtt
            this, &QgsAttributeFormWidget::searchWidgetFlagsChanged );
   l->addWidget( mSearchWidgetToolButton, 0 );
 
-
-  mStack = new QStackedWidget;
-  mStack->addWidget( mEditPage );
-  mStack->addWidget( mSearchPage );
+  mStack = new QStackedWidget();
+  // IMPORTANT!
+  // We do NOT add pages to mStack here, as QStackedWidgets will always inherit the minimum size
+  // of their largest page. This can cause attribute form sizes to needlessly blow out in certain modes,
+  // eg when the form is in the "Add feature" mode we do NOT need the extra horizontal space requirements
+  // that the search widgets enfore. Doing so forces all editor widgets in all modes to have a very wide
+  // minimum width, preventing attribute forms from being shrunk to reasonable sizes without horizontal
+  // scroll bars appearing.
+  // Instead, the pages are added and removed from the stack whenever the visible page is changed (in updateWidgets()).
+  // This ensures that the stack, and this widget too, only inherit the size requirements of the actual visible
+  // page.
 
   l = new QHBoxLayout();
   l->setContentsMargins( 0, 0, 0, 0 );
@@ -63,7 +70,17 @@ QgsAttributeFormWidget::QgsAttributeFormWidget( QgsWidgetWrapper *widget, QgsAtt
   // Respect size policy of embedded widget
   setSizePolicy( mWidget->widget()->sizePolicy() );
 
-  updateWidgets();
+  setVisiblePageForMode( mMode );
+}
+
+QgsAttributeFormWidget::~QgsAttributeFormWidget()
+{
+  // depending on the current page in the stacked widget, these pages NOT
+  // be parented to the stacked widget or this widget. Clean them up manually to avoid leaks.
+  delete mEditPage;
+  mEditPage = nullptr;
+  delete mSearchPage;
+  mSearchPage = nullptr;
 }
 
 void QgsAttributeFormWidget::setMode( QgsAttributeFormWidget::Mode mode )
@@ -170,21 +187,40 @@ void QgsAttributeFormWidget::searchWidgetFlagsChanged( QgsSearchWidgetWrapper::F
 
 void QgsAttributeFormWidget::updateWidgets()
 {
-  switch ( mMode )
+  setVisiblePageForMode( mMode );
+}
+
+void QgsAttributeFormWidget::setVisiblePageForMode( Mode mode )
+{
+  QWidget *currentVisibleWidget = mStack->currentWidget();
+
+  QWidget *newVisibleWidget = nullptr;
+  switch ( mode )
   {
     case DefaultMode:
     case MultiEditMode:
-      mStack->setCurrentWidget( mEditPage );
+      newVisibleWidget = mEditPage;
       break;
 
     case SearchMode:
     case AggregateSearchMode:
     {
-      mStack->setCurrentWidget( mSearchPage );
+      newVisibleWidget = mSearchPage;
       break;
     }
   }
 
+  if ( newVisibleWidget != currentVisibleWidget )
+  {
+    if ( currentVisibleWidget )
+    {
+      // as per Qt docs, this does NOT delete the page, it just removes it from the stack
+      mStack->removeWidget( currentVisibleWidget );
+    }
+
+    mStack->addWidget( newVisibleWidget );
+    mStack->setCurrentWidget( newVisibleWidget );
+  }
 }
 
 bool QgsAttributeFormWidget::searchWidgetToolButtonVisible() const

src/gui/qgsattributeformwidget.h

@@ -55,6 +55,8 @@ class GUI_EXPORT QgsAttributeFormWidget : public QWidget // SIP_ABSTRACT
      */
     explicit QgsAttributeFormWidget( QgsWidgetWrapper *widget, QgsAttributeForm *form );
 
+    ~QgsAttributeFormWidget() override;
+
     /**
      * Creates the search widget wrappers for the widget used when the form is in
      * search mode.
@@ -172,6 +174,13 @@ class GUI_EXPORT QgsAttributeFormWidget : public QWidget // SIP_ABSTRACT
      */
     QWidget *searchPage() const SIP_SKIP;
 
+    /**
+     * Sets the visible page in the widget to the page matching the specified \a mode.
+     *
+     * \since QGIS 3.32
+     */
+    void setVisiblePageForMode( QgsAttributeFormWidget::Mode mode );
+
   private slots:
 
     //! Triggered when search button flags are changed

src/test/qgstest.h

@@ -82,6 +82,15 @@
     QVERIFY( !qgsDoubleNear( value, not_expected, epsilon ) ); \
   }(void)(0)
 
+#define QGSVERIFYLESSTHAN(value,expected) { \
+    bool _xxxresult = value < expected; \
+    if ( !_xxxresult  ) \
+    { \
+      qDebug( "Expecting < %.10f got %.10f", static_cast< double >( expected ), static_cast< double >( value ) ); \
+    } \
+    QVERIFY( value < expected ); \
+  }(void)(0)
+
 #define QGSCOMPARENEARPOINT(point1,point2,epsilon) { \
     QGSCOMPARENEAR( point1.x(), point2.x(), epsilon ); \
     QGSCOMPARENEAR( point1.y(), point2.y(), epsilon ); \

tests/src/gui/testqgsattributeform.cpp

@@ -59,6 +59,7 @@ class TestQgsAttributeForm : public QObject
     void testDefaultValueUpdateRecursion();
     void testSameFieldSync();
     void testZeroDoubles();
+    void testMinimumWidth();
 
   private:
     QLabel *constraintsLabel( QgsAttributeForm *form, QgsEditorWidgetWrapper *ww )
@@ -1164,5 +1165,50 @@ void TestQgsAttributeForm::testZeroDoubles()
   QCOMPARE( les.at( 0 )->text(), QStringLiteral( "0" ) );
 }
 
+void TestQgsAttributeForm::testMinimumWidth()
+{
+  // ensure that the minimum width of editor widgets is as small as possible for the actual attribute form mode.
+  const QString def = QStringLiteral( "Point?field=col0:double" );
+  QgsVectorLayer layer { def, QStringLiteral( "test" ), QStringLiteral( "memory" ) };
+  layer.setEditorWidgetSetup( 0, QgsEditorWidgetSetup( QStringLiteral( "TextEdit" ), QVariantMap() ) );
+  QgsFeature ft( layer.dataProvider()->fields(), 1 );
+  ft.setAttribute( QStringLiteral( "col0" ), 0.0 );
+  QgsAttributeEditorContext context;
+  context.setAttributeFormMode( QgsAttributeEditorContext::SingleEditMode );
+  std::unique_ptr< QgsAttributeForm > form = std::make_unique< QgsAttributeForm >( &layer, QgsFeature(), context );
+  form->setFeature( ft );
+  form->show();
+  // we don't want the larger width requirement of the search wrappers to be enforced when the attribute form
+  // is not in search modes
+  QLineEdit le;
+  const QFontMetrics leMetrics( le.fontMetrics() );
+  QGSVERIFYLESSTHAN( form->minimumWidth(), leMetrics.horizontalAdvance( 'x' ) * 20 );
+
+  form->setMode( QgsAttributeEditorContext::SearchMode );
+  QGSVERIFYLESSTHAN( form->minimumWidth(), leMetrics.horizontalAdvance( 'x' ) * 150 );
+
+  context.setAttributeFormMode( QgsAttributeEditorContext::AddFeatureMode );
+  form = std::make_unique< QgsAttributeForm >( &layer, QgsFeature(), context );
+  form->setFeature( ft );
+  form->show();
+  form->setMode( QgsAttributeEditorContext::AddFeatureMode );
+  QGSVERIFYLESSTHAN( form->minimumWidth(), leMetrics.horizontalAdvance( 'x' ) * 20 );
+
+  context.setAttributeFormMode( QgsAttributeEditorContext::AggregateSearchMode );
+  form = std::make_unique< QgsAttributeForm >( &layer, QgsFeature(), context );
+  form->setFeature( ft );
+  form->show();
+  form->setMode( QgsAttributeEditorContext::AggregateSearchMode );
+  QGSVERIFYLESSTHAN( form->minimumWidth(), leMetrics.horizontalAdvance( 'x' ) * 150 );
+
+  context.setAttributeFormMode( QgsAttributeEditorContext::MultiEditMode );
+  form = std::make_unique< QgsAttributeForm >( &layer, QgsFeature(), context );
+  form->setFeature( ft );
+  form->setMode( QgsAttributeEditorContext::MultiEditMode );
+  form->show();
+  QGSVERIFYLESSTHAN( form->minimumWidth(), leMetrics.horizontalAdvance( 'x' ) * 100 );
+
+}
+
 QGSTEST_MAIN( TestQgsAttributeForm )
 #include "testqgsattributeform.moc"