From d845f9df97274af04ef7cc2dbda7e819b1e91d8a Mon Sep 17 00:00:00 2001
From: Nyall Dawson <nyall.dawson@gmail.com>
Date: Sun, 11 Nov 2018 14:28:11 +1000
Subject: [PATCH] Tweak logic relating to suppressing attribute form for new
 features

For non-spatial layers, creating a new feature from the main
window now ALWAYS shows the attribute form for the new feature,
regardless of the user's "suppress form" setting. We do this because,
unlike for spatial layers, there's zero feedback given when adding
a new feature to a non spatial layer. (Spatial layers have instead
feedback even when the form is suppressed, because you see the
new feature appear on the map instantly)

But when a new feature is added from the attri bute table window,
then we never show the new feature's form -- because that's already
visible inside the attribute table dialog itself.
---
 src/app/qgsattributetabledialog.cpp |  1 +
 src/app/qgsfeatureaction.cpp        | 25 +++++++++++++++++++++++--
 src/app/qgsfeatureaction.h          |  9 +++++++++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/app/qgsattributetabledialog.cpp b/src/app/qgsattributetabledialog.cpp
index 19cbc4bbdfa..0377266c1c2 100644
--- a/src/app/qgsattributetabledialog.cpp
+++ b/src/app/qgsattributetabledialog.cpp
@@ -760,6 +760,7 @@ void QgsAttributeTableDialog::mActionAddFeature_triggered()
 
   QgsFeature f;
   QgsFeatureAction action( tr( "Geometryless feature added" ), f, mLayer, QString(), -1, this );
+  action.setForceSuppressFormPopup( true ); // we're already showing the table, allowing users to enter the new feature's attributes directly
   if ( action.addFeature() )
   {
     masterModel->reload( masterModel->index( 0, 0 ), masterModel->index( masterModel->rowCount() - 1, masterModel->columnCount() - 1 ) );
diff --git a/src/app/qgsfeatureaction.cpp b/src/app/qgsfeatureaction.cpp
index 4821fe9d39a..3e762beaaa6 100644
--- a/src/app/qgsfeatureaction.cpp
+++ b/src/app/qgsfeatureaction.cpp
@@ -200,8 +200,19 @@ bool QgsFeatureAction::addFeature( const QgsAttributeMap &defaultAttributes, boo
   *mFeature = newFeature;
 
   //show the dialog to enter attribute values
-  //only show if enabled in settings and layer has fields
-  bool isDisabledAttributeValuesDlg = ( fields.count() == 0 ) || settings.value( QStringLiteral( "qgis/digitizing/disable_enter_attribute_values_dialog" ), false ).toBool();
+  //only show if enabled in settings
+  bool isDisabledAttributeValuesDlg = settings.value( QStringLiteral( "qgis/digitizing/disable_enter_attribute_values_dialog" ), false ).toBool();
+
+  // override application-wide setting if layer is non-spatial -- BECAUSE it's bad UX if
+  // it appears that nothing happens when you click the add row button for a non-spatial layer. Unlike
+  // spatial layers, where you can SEE the newly created spatial object on the map, creating a new
+  // feature in a non-spatial layer otherwise seems to have no result.
+  if ( !mLayer->isSpatial() )
+    isDisabledAttributeValuesDlg = false;
+
+  // override application-wide setting if layer has no fields
+  if ( fields.count() == 0 )
+    isDisabledAttributeValuesDlg = true;
 
   // override application-wide setting with any layer setting
   switch ( mLayer->editFormConfig().suppress() )
@@ -215,6 +226,11 @@ bool QgsFeatureAction::addFeature( const QgsAttributeMap &defaultAttributes, boo
     case QgsEditFormConfig::SuppressDefault:
       break;
   }
+
+  // finally, if this action has specifically forced suppression of the form, that overrides everything
+  if ( mForceSuppressFormPopup )
+    isDisabledAttributeValuesDlg = true;
+
   if ( isDisabledAttributeValuesDlg )
   {
     mLayer->beginEditCommand( text() );
@@ -255,6 +271,11 @@ bool QgsFeatureAction::addFeature( const QgsAttributeMap &defaultAttributes, boo
   return mFeatureSaved;
 }
 
+void QgsFeatureAction::setForceSuppressFormPopup( bool force )
+{
+  mForceSuppressFormPopup = force;
+}
+
 void QgsFeatureAction::onFeatureSaved( const QgsFeature &feature )
 {
   QgsAttributeForm *form = qobject_cast<QgsAttributeForm *>( sender() );
diff --git a/src/app/qgsfeatureaction.h b/src/app/qgsfeatureaction.h
index 681b552b0cd..7dbf479523f 100644
--- a/src/app/qgsfeatureaction.h
+++ b/src/app/qgsfeatureaction.h
@@ -54,6 +54,13 @@ class APP_EXPORT QgsFeatureAction : public QAction
      */
     bool addFeature( const QgsAttributeMap &defaultAttributes = QgsAttributeMap(), bool showModal = true, QgsExpressionContextScope *scope = nullptr );
 
+    /**
+     * Sets whether to force suppression of the attribute form popup after creating a new feature.
+     * If \a force is true, then regardless of any user settings, form settings, etc, the attribute
+     * form will ALWAYS be suppressed.
+     */
+    void setForceSuppressFormPopup( bool force );
+
   private slots:
     void onFeatureSaved( const QgsFeature &feature );
 
@@ -67,6 +74,8 @@ class APP_EXPORT QgsFeatureAction : public QAction
 
     bool mFeatureSaved;
 
+    bool mForceSuppressFormPopup = false;
+
     static QHash<QgsVectorLayer *, QgsAttributeMap> sLastUsedValues;
 };