Don't go to massive negative numbers when incrementing a NULL spin box

When a spin box is showing a NULL value, we DON'T do the default step behavior, as that would add one step to the NULL value, which is usually a very large negative value... so the user will get a very large negative number! Instead, treat the initial value as 0 instead, and then perform the step.
agiudiceandrea · Jun 28, 2024 · 87d9a3e · 87d9a3e
1 parent f6cef42
commit 87d9a3e
Show file tree

Hide file tree

Showing 10 changed files with 116 additions and 0 deletions.
diff --git a/python/PyQt6/gui/auto_generated/editorwidgets/qgsdoublespinbox.sip.in b/python/PyQt6/gui/auto_generated/editorwidgets/qgsdoublespinbox.sip.in
@@ -135,6 +135,8 @@ is equal to :py:func:`~QgsDoubleSpinBox.minimum`. Typical use is to indicate tha
 
     virtual void paintEvent( QPaintEvent *e );
 
+    virtual void stepBy( int steps );
+
 
   protected:
     virtual void changeEvent( QEvent *event );

diff --git a/python/PyQt6/gui/auto_generated/editorwidgets/qgsspinbox.sip.in b/python/PyQt6/gui/auto_generated/editorwidgets/qgsspinbox.sip.in
@@ -133,6 +133,8 @@ is equal to :py:func:`~QgsSpinBox.minimum`. Typical use is to indicate that this
 
     virtual QValidator::State validate( QString &input, int &pos ) const;
 
+    virtual void stepBy( int steps );
+
 
   protected:
 

diff --git a/python/gui/auto_generated/editorwidgets/qgsdoublespinbox.sip.in b/python/gui/auto_generated/editorwidgets/qgsdoublespinbox.sip.in
@@ -135,6 +135,8 @@ is equal to :py:func:`~QgsDoubleSpinBox.minimum`. Typical use is to indicate tha
 
     virtual void paintEvent( QPaintEvent *e );
 
+    virtual void stepBy( int steps );
+
 
   protected:
     virtual void changeEvent( QEvent *event );

diff --git a/python/gui/auto_generated/editorwidgets/qgsspinbox.sip.in b/python/gui/auto_generated/editorwidgets/qgsspinbox.sip.in
@@ -133,6 +133,8 @@ is equal to :py:func:`~QgsSpinBox.minimum`. Typical use is to indicate that this
 
     virtual QValidator::State validate( QString &input, int &pos ) const;
 
+    virtual void stepBy( int steps );
+
 
   protected:
 

diff --git a/src/gui/editorwidgets/qgsdoublespinbox.cpp b/src/gui/editorwidgets/qgsdoublespinbox.cpp
@@ -114,6 +114,20 @@ void QgsDoubleSpinBox::paintEvent( QPaintEvent *event )
   QDoubleSpinBox::paintEvent( event );
 }
 
+void QgsDoubleSpinBox::stepBy( int steps )
+{
+  const bool wasNull = mShowClearButton && value() == clearValue();
+  if ( wasNull && minimum() < 0 && maximum() > 0 )
+  {
+    // value is currently null, and range allows both positive and negative numbers
+    // in this case we DON'T do the default step, as that would add one step to the NULL value,
+    // which is usually the minimum acceptable value... so the user will get a very large negative number!
+    // Instead, treat the initial value as 0 instead, and then perform the step.
+    whileBlocking( this )->setValue( 0 );
+  }
+  QDoubleSpinBox::stepBy( steps );
+}
+
 void QgsDoubleSpinBox::changed( double value )
 {
   mLineEdit->setShowClearButton( shouldShowClearForValue( value ) );

diff --git a/src/gui/editorwidgets/qgsdoublespinbox.h b/src/gui/editorwidgets/qgsdoublespinbox.h
@@ -140,6 +140,7 @@ class GUI_EXPORT QgsDoubleSpinBox : public QDoubleSpinBox
     double valueFromText( const QString &text ) const override;
     QValidator::State validate( QString &input, int &pos ) const override;
     void paintEvent( QPaintEvent *e ) override;
+    void stepBy( int steps ) override;
 
   protected:
     void changeEvent( QEvent *event ) override;

diff --git a/src/gui/editorwidgets/qgsspinbox.cpp b/src/gui/editorwidgets/qgsspinbox.cpp
@@ -216,6 +216,20 @@ QValidator::State QgsSpinBox::validate( QString &input, int &pos ) const
   return QValidator::Acceptable;
 }
 
+void QgsSpinBox::stepBy( int steps )
+{
+  const bool wasNull = mShowClearButton && value() == clearValue();
+  if ( wasNull && minimum() < 0 && maximum() > 0 )
+  {
+    // value is currently null, and range allows both positive and negative numbers
+    // in this case we DON'T do the default step, as that would add one step to the NULL value,
+    // which is usually the minimum acceptable value... so the user will get a very large negative number!
+    // Instead, treat the initial value as 0 instead, and then perform the step.
+    whileBlocking( this )->setValue( 0 );
+  }
+  QSpinBox::stepBy( steps );
+}
+
 int QgsSpinBox::frameWidth() const
 {
   return style()->pixelMetric( QStyle::PM_DefaultFrameWidth );

diff --git a/src/gui/editorwidgets/qgsspinbox.h b/src/gui/editorwidgets/qgsspinbox.h
@@ -139,6 +139,7 @@ class GUI_EXPORT QgsSpinBox : public QSpinBox
 
     int valueFromText( const QString &text ) const override;
     QValidator::State validate( QString &input, int &pos ) const override;
+    void stepBy( int steps ) override;
 
   protected:
 

diff --git a/tests/src/gui/testqgsdoublespinbox.cpp b/tests/src/gui/testqgsdoublespinbox.cpp
@@ -29,6 +29,7 @@ class TestQgsDoubleSpinBox: public QObject
 
     void clear();
     void expression();
+    void step();
 
   private:
 
@@ -144,5 +145,43 @@ void TestQgsDoubleSpinBox::expression()
   delete spinBox;
 }
 
+void TestQgsDoubleSpinBox::step()
+{
+  // test step logic
+
+  QgsDoubleSpinBox spin;
+  spin.setMinimum( -1000 );
+  spin.setMaximum( 1000 );
+  spin.setSingleStep( 1 );
+
+  // no clear value
+  spin.setValue( 0 );
+  spin.stepBy( 1 );
+  QCOMPARE( spin.value(), 1 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), 0 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), -1 );
+
+  // with clear value
+  spin.setClearValue( -1000, QStringLiteral( "NULL" ) );
+  spin.setValue( 0 );
+  spin.stepBy( 1 );
+  QCOMPARE( spin.value(), 1 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), 0 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), -1 );
+  spin.clear();
+  QCOMPARE( spin.value(), -1000 );
+  // when cleared, a step should NOT go to -999 (which is annoying for users), but rather pretend that the initial value was 0, not NULL
+  spin.stepBy( 1 );
+  QCOMPARE( spin.value(), 1 );
+  spin.clear();
+  QCOMPARE( spin.value(), -1000 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), -1 );
+}
+
 QGSTEST_MAIN( TestQgsDoubleSpinBox )
 #include "testqgsdoublespinbox.moc"
diff --git a/tests/src/gui/testqgsspinbox.cpp b/tests/src/gui/testqgsspinbox.cpp
@@ -29,6 +29,7 @@ class TestQgsSpinBox: public QObject
 
     void clear();
     void expression();
+    void step();
 
   private:
 
@@ -144,5 +145,43 @@ void TestQgsSpinBox::expression()
   delete spinBox;
 }
 
+void TestQgsSpinBox::step()
+{
+  // test step logic
+
+  QgsSpinBox spin;
+  spin.setMinimum( -1000 );
+  spin.setMaximum( 1000 );
+  spin.setSingleStep( 1 );
+
+  // no clear value
+  spin.setValue( 0 );
+  spin.stepBy( 1 );
+  QCOMPARE( spin.value(), 1 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), 0 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), -1 );
+
+  // with clear value
+  spin.setClearValue( -1000, QStringLiteral( "NULL" ) );
+  spin.setValue( 0 );
+  spin.stepBy( 1 );
+  QCOMPARE( spin.value(), 1 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), 0 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), -1 );
+  spin.clear();
+  QCOMPARE( spin.value(), -1000 );
+  // when cleared, a step should NOT go to -999 (which is annoying for users), but rather pretend that the initial value was 0, not NULL
+  spin.stepBy( 1 );
+  QCOMPARE( spin.value(), 1 );
+  spin.clear();
+  QCOMPARE( spin.value(), -1000 );
+  spin.stepBy( -1 );
+  QCOMPARE( spin.value(), -1 );
+}
+
 QGSTEST_MAIN( TestQgsSpinBox )
 #include "testqgsspinbox.moc"
Original file line number	Diff line number	Diff line change
Expand Up		@@ -133,6 +133,8 @@ is equal to :py:func:`~QgsSpinBox.minimum`. Typical use is to indicate that this

		virtual QValidator::State validate( QString &input, int &pos ) const;

		virtual void stepBy( int steps );


		protected:

Expand Down