Merge pull request #240 from homann/test_scalecombobox

Test and refactoring - qgsscalecombobox

python/gui/qgsscalecombobox.sip

@@ -12,6 +12,28 @@ class QgsScaleComboBox : QComboBox
     QgsScaleComboBox(QWidget * parent = 0);
     ~QgsScaleComboBox();
 
+    //! Function to read the selected scale as text
+    // @note added in 2.0
+    QString scaleString();
+    //! Function to set the selected scale from text
+    // @note added in 2.0
+    bool setScaleString( QString scaleTxt );
+    //! Function to read the selected scale as double
+    // @note added in 2.0
+    double scale();
+    //! Function to set the selected scale from double
+    // @note added in 2.0
+    void setScale( double scale );
+ 
+   //! Helper function to convert a double to scale string
+    // Performs rounding, so an exact representation is not to
+    // be expected.
+    // @note added in 2.0
+    static QString toString( double scale );
+    //! Helper function to convert a scale string to double
+    // @note added in 2.0
+    static double toDouble( QString scaleString, bool *ok = NULL );
+
   public slots:
     void updateScales( const QStringList &scales = QStringList() );
 };

src/app/qgisapp.cpp

@@ -1467,16 +1467,11 @@ void QgisApp::createStatusBar()
   mScaleEdit->setMaximumWidth( 100 );
   mScaleEdit->setMaximumHeight( 20 );
   mScaleEdit->setContentsMargins( 0, 0, 0, 0 );
-  // QRegExp validator( "\\d+\\.?\\d*:\\d+\\.?\\d*" );
-  QRegExp validator( "\\d+\\.?\\d*:\\d+\\.?\\d*|\\d+\\.?\\d*" );
-  mScaleEditValidator = new QRegExpValidator( validator, mScaleEdit );
-  mScaleEdit->setValidator( mScaleEditValidator );
   mScaleEdit->setWhatsThis( tr( "Displays the current map scale" ) );
   mScaleEdit->setToolTip( tr( "Current map scale (formatted as x:y)" ) );
 
   statusBar()->addPermanentWidget( mScaleEdit, 0 );
-  connect( mScaleEdit, SIGNAL( currentIndexChanged( const QString & ) ), this, SLOT( userScale() ) );
-  connect( mScaleEdit->lineEdit(), SIGNAL( editingFinished() ), this, SLOT( userScale() ) );
+  connect( mScaleEdit, SIGNAL( scaleChanged() ), this, SLOT( userScale() ) );
 
   //stop rendering status bar widget
   mStopRenderButton = new QToolButton( statusBar() );
@@ -5249,15 +5244,10 @@ void QgisApp::showMouseCoordinate( const QgsPoint & p )
 
 void QgisApp::showScale( double theScale )
 {
-  if ( theScale >= 1.0 )
-    mScaleEdit->setEditText( "1:" + QString::number( theScale, 'f', 0 ) );
-  else if ( theScale > 0.0 )
-    mScaleEdit->setEditText( QString::number( 1.0 / theScale, 'f', 0 ) + ":1" );
-  else
-    mScaleEdit->setEditText( tr( "Invalid scale" ) );
-
-  mOldScale = mScaleEdit->currentText();
+  // Why has MapCanvas the scale inverted?
+  mScaleEdit->setScale( 1.0 / theScale );
 
+  // Not sure if the lines below do anything meaningful /Homann
   if ( mScaleEdit->width() > mScaleEdit->minimumWidth() )
   {
     mScaleEdit->setMinimumWidth( mScaleEdit->width() );
@@ -5266,31 +5256,8 @@ void QgisApp::showScale( double theScale )
 
 void QgisApp::userScale()
 {
-  if ( mOldScale == mScaleEdit->currentText() )
-  {
-    return;
-  }
-
-  QStringList parts = mScaleEdit->currentText().split( ':' );
-  if ( parts.size() == 2 )
-  {
-    bool leftOk, rightOk;
-    double leftSide = parts.at( 0 ).toDouble( &leftOk );
-    double rightSide = parts.at( 1 ).toDouble( &rightOk );
-    if ( leftSide > 0.0 && leftOk && rightOk )
-    {
-      mMapCanvas->zoomScale( rightSide / leftSide );
-    }
-  }
-  else
-  {
-    bool rightOk;
-    double rightSide = parts.at( 0 ).toDouble( &rightOk );
-    if ( rightOk )
-    {
-      mMapCanvas->zoomScale( rightSide );
-    }
-  }
+  // Why has MapCanvas the scale inverted?
+  mMapCanvas->zoomScale( 1.0 / mScaleEdit->scale() );
 }
 
 void QgisApp::userCenter()

src/app/qgisapp.h

@@ -1236,8 +1236,6 @@ class QgisApp : public QMainWindow, private Ui::MainWindow
 
     bool cmpByText( QAction* a, QAction* b );
 
-    QString mOldScale;
-
     //! the user has trusted the project macros
     bool mTrustedMacros;
 

src/gui/qgsscalecombobox.cpp

@@ -16,10 +16,13 @@
  ***************************************************************************/
 
 #include "qgis.h"
+#include "qgslogger.h"
 #include "qgsscalecombobox.h"
 
 #include <QAbstractItemView>
+#include <QLocale>
 #include <QSettings>
+#include <QLineEdit>
 
 QgsScaleComboBox::QgsScaleComboBox( QWidget* parent ) : QComboBox( parent )
 {
@@ -28,6 +31,9 @@ QgsScaleComboBox::QgsScaleComboBox( QWidget* parent ) : QComboBox( parent )
   setEditable( true );
   setInsertPolicy( QComboBox::NoInsert );
   setCompleter( 0 );
+  connect( this, SIGNAL( activated( const QString & ) ), this, SLOT( fixupScale() ) );
+  connect( lineEdit(), SIGNAL( editingFinished() ), this, SLOT( fixupScale() ) );
+  fixupScale();
 }
 
 QgsScaleComboBox::~QgsScaleComboBox()
@@ -60,7 +66,7 @@ void QgsScaleComboBox::updateScales( const QStringList &scales )
   blockSignals( true );
   clear();
   addItems( myScalesList );
-  setEditText( oldScale );
+  setScaleString( oldScale );
   blockSignals( false );
 }
 
@@ -94,3 +100,122 @@ void QgsScaleComboBox::showPopup()
   view()->setCurrentIndex( model()->index( idx, 0 ) );
   blockSignals( false );
 }
+
+//! Function to read the selected scale as text
+// @note added in 2.0
+QString QgsScaleComboBox::scaleString()
+{
+  return toString( mScale );
+}
+
+//! Function to set the selected scale from text
+// @note added in 2.0
+bool QgsScaleComboBox::setScaleString( QString scaleTxt )
+{
+  bool ok;
+  double newScale = toDouble( scaleTxt, &ok );
+  if ( ! ok )
+  {
+    return false;
+  }
+  else
+  {
+    mScale = newScale;
+    setEditText( toString( mScale ) );
+    clearFocus();
+    return true;
+  }
+}
+
+//! Function to read the selected scale as double
+// @note added in 2.0
+double QgsScaleComboBox::scale()
+{
+  return mScale;
+}
+
+//! Function to set the selected scale from double
+// @note added in 2.0
+void QgsScaleComboBox::setScale( double scale )
+{
+  setScaleString( toString( scale ) );
+}
+
+//! Slot called when QComboBox has changed
+void QgsScaleComboBox::fixupScale()
+{
+  double newScale;
+  double oldScale = mScale;
+  bool ok;
+  QStringList txtList;
+
+  // QgsDebugMsg( QString( "entered with oldScale: %1" ).arg( oldScale ) );
+  newScale = toDouble( currentText(), &ok );
+  if ( ok )
+  {
+    // Valid string representation
+    if ( newScale != oldScale )
+    {
+      // Scale has change, update.
+      // QgsDebugMsg( QString( "New scale OK!: %1" ).arg( newScale ) );
+      mScale = newScale;
+      setScale( mScale );
+      emit scaleChanged();
+    }
+  }
+  else
+  {
+    // Invalid string representation
+    // Reset to the old
+    setScale( mScale );
+  }
+}
+
+QString QgsScaleComboBox::toString( double scale )
+{
+  if ( scale > 1 )
+  {
+    return QString( "%1:1" ).arg( qRound( scale ) );
+  }
+  else
+  {
+    return QString( "1:%1" ).arg( qRound( 1.0 / scale ) );
+  }
+}
+
+double QgsScaleComboBox::toDouble( QString scaleString, bool * returnOk )
+{
+  bool ok = false;
+  QString scaleTxt( scaleString );
+
+  double scale = QLocale::system().toDouble( scaleTxt, &ok );
+  if ( ok )
+  {
+    // Create a text version and set that text and rescan
+    // Idea is to get the same rounding.
+    scaleTxt = toString( scale );
+  }
+  // It is now either X:Y or not valid
+  ok = false;
+  QStringList txtList = scaleTxt.split( ':' );
+  if ( 2 == txtList.size() )
+  {
+    bool okX = false;
+    bool okY = false;
+    int x = QLocale::system().toInt( txtList[ 0 ], &okX );
+    int y = QLocale::system().toInt( txtList[ 1 ], &okY );
+    if ( okX && okY )
+    {
+      // Scale is fraction of x and y
+      scale = ( double )x / ( double )y;
+      ok = true;
+    }
+  }
+
+  // Set up optional return flag
+  if ( returnOk )
+  {
+    *returnOk = ok;
+  }
+  return scale;
+}

src/gui/qgsscalecombobox.h

@@ -30,12 +30,45 @@ class GUI_EXPORT QgsScaleComboBox : public QComboBox
   public:
     QgsScaleComboBox( QWidget* parent = 0 );
     virtual ~QgsScaleComboBox();
+    //! Function to read the selected scale as text
+    // @note added in 2.0
+    QString scaleString();
+    //! Function to set the selected scale from text
+    // @note added in 2.0
+    bool setScaleString( QString scaleTxt );
+    //! Function to read the selected scale as double
+    // @note added in 2.0
+    double scale();
+    //! Function to set the selected scale from double
+    // @note added in 2.0
+    void setScale( double scale );
+
+    //! Helper function to convert a double to scale string
+    // Performs rounding, so an exact representation is not to
+    // be expected.
+    // @note added in 2.0
+    static QString toString( double scale );
+    //! Helper function to convert a scale string to double
+    // @note added in 2.0
+    static double toDouble( QString scaleString, bool *ok = NULL );
+
+  signals:
+    //! Signal is emitted when *user* has finished editing/selecting a new scale.
+    // @note added in 2.0
+    void scaleChanged();
 
   public slots:
     void updateScales( const QStringList &scales = QStringList() );
 
+
   protected:
     void showPopup();
+
+  private slots:
+    void fixupScale();
+
+  private:
+    double mScale;
 };
 
 #endif // QGSSCALECOMBOBOX_H

tests/src/gui/CMakeLists.txt

@@ -119,4 +119,5 @@ ADD_QGIS_TEST(zoomtest testqgsmaptoolzoom.cpp)
 
 ADD_QGIS_TEST(histogramtest testqgsrasterhistogram.cpp)
 ADD_QGIS_TEST(projectionissues testprojectionissues.cpp)
+ADD_QGIS_TEST(scalecombobox testqgsscalecombobox.cpp)
 

tests/src/gui/testqgsscalecombobox.cpp

@@ -0,0 +1,133 @@
+/***************************************************************************
+                         testqgsscalecombobox.cpp
+                         ---------------------------
+    begin                : September 2012
+    copyright            : (C) 2012 by Magnus Homann
+    email                : magnus at homann dot se
+ ***************************************************************************/
+
+/***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+
+#include "qgsapplication.h"
+#include "qgslogger.h"
+#include "qgsscalecombobox.h"
+#include <QObject>
+#include <QLineEdit>
+#include <QComboBox>
+#include <QSignalSpy>
+#include <QtTest>
+
+class TestQgsScaleComboBox: public QObject
+{
+    Q_OBJECT;
+  private slots:
+    void initTestCase();// will be called before the first testfunction is executed.
+    void cleanupTestCase();// will be called after the last testfunction was executed.
+    void init();// will be called before each testfunction is executed.
+    void cleanup();// will be called after every testfunction.
+    void basic();
+    void slot_test();
+  private:
+    QgsScaleComboBox *s;
+};
+
+void TestQgsScaleComboBox::initTestCase()
+{
+  QgsApplication::init();
+  QgsApplication::initQgis();
+
+  // Create a combobox, and init with predefined scales.
+  s = new QgsScaleComboBox();
+  QgsDebugMsg( QString( "Initial scale is %1" ).arg( s->scaleString() ) );
+};
+
+void TestQgsScaleComboBox::cleanupTestCase()
+{
+  delete s;
+};
+
+void TestQgsScaleComboBox::init()
+{
+};
+
+void TestQgsScaleComboBox::basic()
+{
+  QLineEdit *l = s->lineEdit();
+
+  // Testing conversion from "1:nnn".
+  l->setText( "" );
+  QTest::keyClicks( l, "1:2345" );
+  QTest::keyClick( l, Qt::Key_Return );
+  QCOMPARE( s->scaleString(), QString( "1:2345" ) );
+  QCOMPARE( s->scale(), (( double ) 1.0 / ( double ) 2345.0 ) );
+
+  // Testing conversion from number to "1:x"
+  l->setText( "" );
+  QTest::keyClicks( l, QLocale::system().toString( 0.02 ) );
+  QTest::keyClick( l, Qt::Key_Return );
+  QCOMPARE( s->scaleString(), QString( "1:50" ) );
+  QCOMPARE( s->scale(), ( double ) 0.02 );
+
+  // Testing conversion from number to "x:1"
+  l->setText( "" );
+  QTest::keyClicks( l, QLocale::system().toString( 42 ) );
+  QTest::keyClick( l, Qt::Key_Return );
+  QCOMPARE( s->scaleString(), QString( "42:1" ) );
+  QCOMPARE( s->scale(), ( double ) 42 );
+
+  // Testing rounding and conversion from illegal
+
+  l->setText( "" );
+  QTest::keyClicks( l, QLocale::system().toString( 0.24 ) );
+  QTest::keyClick( l, Qt::Key_Return );
+
+  l->setText( "" );
+  QTest::keyClicks( l, "1:x:2" );
+  QTest::keyClick( l, Qt::Key_Return );
+  QCOMPARE( s->scaleString(), QString( "1:4" ) );
+  QCOMPARE( s->scale(), ( double ) 0.25 );
+
+  // Test setting programatically
+  s->setScale(( double ) 0.19 );
+  QCOMPARE( s->scaleString(), QString( "1:5" ) );
+  QCOMPARE( s->scale(), ( double ) 0.2 );
+
+  // Test setting programatically
+  s->setScaleString( QString( "1:240" ) );
+  QCOMPARE( s->scaleString(), QString( "1:240" ) );
+  QCOMPARE( s->scale(), ( double ) 1.0 / ( double ) 240.0 );
+
+  // Test setting programatically illegal string
+  s->setScaleString( QString( "1:2.4" ) );
+  QCOMPARE( s->scaleString(), QString( "1:240" ) );
+  QCOMPARE( s->scale(), ( double ) 1.0 / ( double ) 240.0 );
+
+};
+
+void TestQgsScaleComboBox::slot_test()
+{
+  QLineEdit *l = s->lineEdit();
+  l->setText( "" );
+
+  QSignalSpy spyScaleChanged( s, SIGNAL( scaleChanged() ) );
+  QSignalSpy spyFixup( l, SIGNAL( editingFinished() ) );
+
+  QTest::keyClicks( l, QLocale::system().toString( 0.02 ) );
+  QTest::keyClick( l, Qt::Key_Return );
+  QCOMPARE( spyFixup.count(), 2 ); // Qt emits twice!?
+  QCOMPARE( spyScaleChanged.count(), 1 );
+}
+
+void TestQgsScaleComboBox::cleanup()
+{
+};
+
+QTEST_MAIN( TestQgsScaleComboBox )
+#include "moc_testqgsscalecombobox.cxx"