[needs-docs] Move label obstacle settings to "placement" tab

and show detailed settings in a stacked panel widget Because: 1. The settings belong next to the placement settings, because they affect label placement directly (and are closely associated with the label priority setting in this tab) 2. The label settings widget is ridiculously complex and overwhelming even for experienced QGIS users. By moving detailed settings into stacked panels we can avoid the initial complexity of the label settings whilst also allowing us more flexibility to expose additional settings in future without adding to the mess. So in future, I propose we move all detailed settings to sub panels, and leave only the initial "enable" setting at the top level (e.g. [x] "Repeat levels" -> click settings -> see all repeat settings).
qgis · Dec 4, 2019 · 02fffdc · 02fffdc
1 parent cf43104
commit 02fffdc
Show file tree

Hide file tree

Showing 8 changed files with 183 additions and 153 deletions.
diff --git a/python/gui/auto_generated/qgslabelsettingswidgetbase.sip.in b/python/gui/auto_generated/qgslabelsettingswidgetbase.sip.in
@@ -9,6 +9,7 @@
 
 
 
+
 class QgsLabelSettingsWidgetBase : QgsPanelWidget, protected QgsExpressionContextGenerator
 {
 %Docstring
@@ -84,6 +85,30 @@ and help text for the associated property.
 
 };
 
+class QgsLabelSettingsWidgetDialog : QDialog
+{
+%Docstring
+A blocking dialog containing a QgsLabelSettingsWidgetBase.
+
+.. versionadded:: 3.12
+%End
+
+%TypeHeaderCode
+#include "qgslabelsettingswidgetbase.h"
+%End
+  public:
+
+
+    QgsLabelSettingsWidgetDialog( QgsLabelSettingsWidgetBase *widget /Transfer/, QWidget *parent = 0 );
+%Docstring
+Constructor for QgsLabelSettingsWidgetDialog.
+
+:param widget: label settings widget to embed in the dialog. Ownership is transferred to the dialog.
+:param parent: parent widget
+%End
+
+};
+
 /************************************************************************
  * This file has been generated automatically from                      *
  *                                                                      *

diff --git a/src/gui/qgslabelinggui.cpp b/src/gui/qgslabelinggui.cpp
@@ -30,6 +30,7 @@
 #include "qgsapplication.h"
 #include "qgscalloutsregistry.h"
 #include "callouts/qgscalloutwidget.h"
+#include "qgslabelobstaclesettingswidget.h"
 #include <mutex>
 
 #include <QButtonGroup>
@@ -125,6 +126,49 @@ void QgsLabelingGui::updateCalloutWidget( QgsCallout *callout )
   mCalloutStackedWidget->setCurrentWidget( pageDummy );
 }
 
+void QgsLabelingGui::showObstacleSettings()
+{
+  QgsExpressionContext context = createExpressionContext();
+
+  QgsSymbolWidgetContext symbolContext;
+  symbolContext.setExpressionContext( &context );
+  symbolContext.setMapCanvas( mMapCanvas );
+
+  QgsLabelObstacleSettingsWidget *widget = new QgsLabelObstacleSettingsWidget( nullptr, mLayer );
+  widget->setDataDefinedProperties( mDataDefinedProperties );
+  widget->setObstacleSettings( mObstacleSettings );
+  widget->setGeometryType( mLayer ? mLayer->geometryType() : QgsWkbTypes::UnknownGeometry );
+  widget->setContext( symbolContext );
+
+  auto applySettings = [ = ]
+  {
+    mObstacleSettings = widget->settings();
+    const QgsPropertyCollection obstacleDataDefinedProperties = widget->dataDefinedProperties();
+    mDataDefinedProperties.setProperty( QgsPalLayerSettings::ObstacleFactor, obstacleDataDefinedProperties.property( QgsPalLayerSettings::ObstacleFactor ) );
+    emit widgetChanged();
+  };
+
+  QgsPanelWidget *panel = QgsPanelWidget::findParentPanel( this );
+  if ( panel && panel->dockMode() )
+  {
+    connect( widget, &QgsLabelSettingsWidgetBase::changed, this, [ = ]
+    {
+      applySettings();
+    } );
+    panel->openPanel( widget );
+  }
+  else
+  {
+    QgsLabelSettingsWidgetDialog dialog( widget, this );
+    if ( dialog.exec() )
+    {
+      applySettings();
+    }
+    // reactivate button's window
+    activateWindow();
+  }
+}
+
 QgsLabelingGui::QgsLabelingGui( QgsVectorLayer *layer, QgsMapCanvas *mapCanvas, const QgsPalLayerSettings &layerSettings, QWidget *parent, QgsWkbTypes::GeometryType geomType )
   : QgsTextFormatWidget( mapCanvas, parent, QgsTextFormatWidget::Labeling, layer )
   , mGeomType( geomType )
@@ -153,6 +197,7 @@ QgsLabelingGui::QgsLabelingGui( QgsVectorLayer *layer, QgsMapCanvas *mapCanvas,
   connect( mGeometryGeneratorGroupBox, &QGroupBox::toggled, this, &QgsLabelingGui::validateGeometryGeneratorExpression );
   connect( mGeometryGenerator, &QgsCodeEditorExpression::textChanged, this, &QgsLabelingGui::validateGeometryGeneratorExpression );
   connect( mGeometryGeneratorType, qgis::overload<int>::of( &QComboBox::currentIndexChanged ), this, &QgsLabelingGui::validateGeometryGeneratorExpression );
+  connect( mObstacleSettingsButton, &QAbstractButton::clicked, this, &QgsLabelingGui::showObstacleSettings );
 
   mFieldExpressionWidget->registerExpressionContextGenerator( this );
 
@@ -288,8 +333,9 @@ void QgsLabelingGui::setLayer( QgsMapLayer *mapLayer )
 
   mPrioritySlider->setValue( mSettings.priority );
   mChkNoObstacle->setChecked( mSettings.obstacleSettings().isObstacle() );
-  mObstacleFactorSlider->setValue( mSettings.obstacleSettings().factor() * 5 );
-  mObstacleTypeComboBox->setCurrentIndex( mObstacleTypeComboBox->findData( mSettings.obstacleSettings().type() ) );
+
+  mObstacleSettings = mSettings.obstacleSettings();
+
   mPolygonObstacleTypeFrame->setEnabled( mSettings.obstacleSettings().isObstacle() );
   mObstaclePriorityFrame->setEnabled( mSettings.obstacleSettings().isObstacle() );
   chkLabelPerFeaturePart->setChecked( mSettings.labelPerPart );
@@ -488,9 +534,10 @@ QgsPalLayerSettings QgsLabelingGui::layerSettings()
   lyr.overrunDistanceMapUnitScale = mOverrunDistanceUnitWidget->getMapUnitScale();
 
   lyr.priority = mPrioritySlider->value();
-  lyr.obstacleSettings().setIsObstacle( mChkNoObstacle->isChecked() || mMode == ObstaclesOnly );
-  lyr.obstacleSettings().setFactor( mObstacleFactorSlider->value() / 5.0 );
-  lyr.obstacleSettings().setType( static_cast< QgsLabelObstacleSettings::ObstacleType >( mObstacleTypeComboBox->currentData().toInt() ) );
+
+  mObstacleSettings.setIsObstacle( mChkNoObstacle->isChecked() || mMode == ObstaclesOnly );
+  lyr.setObstacleSettings( mObstacleSettings );
+
   lyr.labelPerPart = chkLabelPerFeaturePart->isChecked();
   lyr.displayAll = mPalShowAllLabelsForLayerChkBx->isChecked();
   lyr.mergeLines = chkMergeLines->isChecked();

diff --git a/src/gui/qgslabelinggui.h b/src/gui/qgslabelinggui.h
@@ -91,12 +91,15 @@ class GUI_EXPORT QgsLabelingGui : public QgsTextFormatWidget
     QgsFeature mPreviewFeature;
     QgsMapCanvas *mCanvas = nullptr;
 
+    QgsLabelObstacleSettings mObstacleSettings;
+
     QgsExpressionContext createExpressionContext() const override;
 
   private slots:
 
     void initCalloutWidgets();
     void updateCalloutWidget( QgsCallout *callout );
+    void showObstacleSettings();
 
 };
 

diff --git a/src/gui/qgslabelobstaclesettingswidget.cpp b/src/gui/qgslabelobstaclesettingswidget.cpp
@@ -22,6 +22,8 @@ QgsLabelObstacleSettingsWidget::QgsLabelObstacleSettingsWidget( QWidget *parent,
 {
   setupUi( this );
 
+  setPanelTitle( tr( "Obstacle Settings" ) );
+
   mObstacleTypeComboBox->addItem( tr( "Over the Feature's Interior" ), QgsLabelObstacleSettings::PolygonInterior );
   mObstacleTypeComboBox->addItem( tr( "Over the Feature's Boundary" ), QgsLabelObstacleSettings::PolygonBoundary );
 
@@ -35,6 +37,9 @@ QgsLabelObstacleSettingsWidget::QgsLabelObstacleSettingsWidget( QWidget *parent,
     if ( !mBlockSignals )
       emit changed();
   } );
+
+  registerDataDefinedButton( mObstacleFactorDDBtn, QgsPalLayerSettings::ObstacleFactor );
+
 }
 
 void QgsLabelObstacleSettingsWidget::setObstacleSettings( const QgsLabelObstacleSettings &settings )
@@ -55,6 +60,6 @@ QgsLabelObstacleSettings QgsLabelObstacleSettingsWidget::settings() const
 
 void QgsLabelObstacleSettingsWidget::setGeometryType( QgsWkbTypes::GeometryType type )
 {
-  mObstacleTypeComboBox->setVisible( type == QgsWkbTypes::PolygonGeometry );
-  mObstacleTypeComboBox->setVisible( type == QgsWkbTypes::PolygonGeometry );
+  mObstacleTypeComboBox->setVisible( type == QgsWkbTypes::PolygonGeometry || type == QgsWkbTypes::UnknownGeometry );
+  mObstacleTypeLabel->setVisible( type == QgsWkbTypes::PolygonGeometry || type == QgsWkbTypes::UnknownGeometry );
 }
diff --git a/src/gui/qgslabelsettingswidgetbase.cpp b/src/gui/qgslabelsettingswidgetbase.cpp
@@ -20,6 +20,7 @@
 #include "qgsnewauxiliarylayerdialog.h"
 #include "qgspropertyoverridebutton.h"
 #include "qgsauxiliarystorage.h"
+#include "qgsgui.h"
 
 
 QgsLabelSettingsWidgetBase::QgsLabelSettingsWidgetBase( QWidget *parent, QgsVectorLayer *vl )
@@ -142,3 +143,24 @@ void QgsLabelSettingsWidgetBase::registerDataDefinedButton( QgsPropertyOverrideB
 
   button->registerExpressionContextGenerator( this );
 }
+
+
+//
+// QgsLabelSettingsWidgetDialog
+//
+
+QgsLabelSettingsWidgetDialog::QgsLabelSettingsWidgetDialog( QgsLabelSettingsWidgetBase *widget, QWidget *parent )
+  : QDialog( parent )
+{
+  setWindowTitle( widget->windowTitle() );
+  QVBoxLayout *vLayout = new QVBoxLayout();
+  vLayout->addWidget( widget );
+  QDialogButtonBox *bbox = new QDialogButtonBox( QDialogButtonBox::Ok | QDialogButtonBox::Cancel, Qt::Horizontal );
+  connect( bbox, &QDialogButtonBox::accepted, this, &QDialog::accept );
+  connect( bbox, &QDialogButtonBox::rejected, this, &QDialog::reject );
+  vLayout->addWidget( bbox );
+  setLayout( vLayout );
+
+  setObjectName( QStringLiteral( "QgsLabelSettingsWidgetDialog" ) );
+  QgsGui::instance()->enableAutoGeometryRestore( this );
+}
diff --git a/src/gui/qgslabelsettingswidgetbase.h b/src/gui/qgslabelsettingswidgetbase.h
@@ -24,6 +24,8 @@
 #include "qgis_gui.h"
 #include "qgis_sip.h"
 
+#include <QDialog>
+
 class QgsPropertyOverrideButton;
 
 /**
@@ -111,4 +113,26 @@ class GUI_EXPORT QgsLabelSettingsWidgetBase : public QgsPanelWidget, protected Q
 
 };
 
+/**
+ * \ingroup gui
+ * \class QgsLabelSettingsDialog
+ * A blocking dialog containing a QgsLabelSettingsWidgetBase.
+ * \since QGIS 3.12
+ */
+class GUI_EXPORT QgsLabelSettingsWidgetDialog : public QDialog
+{
+    Q_OBJECT
+
+  public:
+
+
+    /**
+     * Constructor for QgsLabelSettingsWidgetDialog.
+     * \param widget label settings widget to embed in the dialog. Ownership is transferred to the dialog.
+     * \param parent parent widget
+     */
+    QgsLabelSettingsWidgetDialog( QgsLabelSettingsWidgetBase *widget SIP_TRANSFER, QWidget *parent = nullptr );
+
+};
+
 #endif // QGSLABELSETTINGSWIDGETBASE_H
diff --git a/src/gui/qgstextformatwidget.cpp b/src/gui/qgstextformatwidget.cpp
@@ -169,9 +169,6 @@ void QgsTextFormatWidget::initWidget()
   mZIndexSpinBox->setClearValue( 0.0 );
   mLineDistanceSpnBx->setClearValue( 0.0 );
 
-  mObstacleTypeComboBox->addItem( tr( "Over the Feature's Interior" ), QgsLabelObstacleSettings::PolygonInterior );
-  mObstacleTypeComboBox->addItem( tr( "Over the Feature's Boundary" ), QgsLabelObstacleSettings::PolygonBoundary );
-
   mOffsetTypeComboBox->addItem( tr( "From Point" ), QgsPalLayerSettings::FromPoint );
   mOffsetTypeComboBox->addItem( tr( "From Symbol Bounds" ), QgsPalLayerSettings::FromSymbolBounds );
 
@@ -388,8 +385,6 @@ void QgsTextFormatWidget::initWidget()
           << mMaxCharAngleInDSpinBox
           << mMaxCharAngleOutDSpinBox
           << mMinSizeSpinBox
-          << mObstacleFactorSlider
-          << mObstacleTypeComboBox
           << mOffsetTypeComboBox
           << mPalShowAllLabelsForLayerChkBx
           << mPointAngleSpinBox
@@ -798,7 +793,6 @@ void QgsTextFormatWidget::populateDataDefinedButtons()
   registerDataDefinedButton( mAlwaysShowDDBtn, QgsPalLayerSettings::AlwaysShow );
 
   registerDataDefinedButton( mIsObstacleDDBtn, QgsPalLayerSettings::IsObstacle );
-  registerDataDefinedButton( mObstacleFactorDDBtn, QgsPalLayerSettings::ObstacleFactor );
   registerDataDefinedButton( mZIndexDDBtn, QgsPalLayerSettings::ZIndex );
 
   registerDataDefinedButton( mCalloutDrawDDBtn, QgsPalLayerSettings::CalloutDraw );