Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[bugfix] Allow empty null representation in spinboxes #7943

Merged
merged 2 commits into from Sep 18, 2018
Merged

[bugfix] Allow empty null representation in spinboxes #7943

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d179635
[bugfix] Allow empty null representation in spinboxes
elpaso Sep 18, 2018
0edb224
Fix QCOMPARE in the test
elpaso Sep 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions python/gui/auto_generated/editorwidgets/qgsdoublespinbox.sip.in
View file
Open in desktop
Expand Up @@ -124,6 +124,13 @@ Returns the value used when clear() is called.
Set alignment in the embedded line edit widget

:param alignment:
%End

void setSpecialValueText( const QString &txt );
%Docstring
Set the special-value text to be ``txt``
If set, the spin box will display this text instead of a numeric value whenever the current value
is equal to minimum(). Typical use is to indicate that this choice has a special (default) meaning.
%End

virtual double valueFromText( const QString &text ) const;
Expand Down
7 changes: 7 additions & 0 deletions python/gui/auto_generated/editorwidgets/qgsspinbox.sip.in
View file
Open in desktop
Expand Up @@ -124,6 +124,13 @@ Returns the value used when clear() is called.
Set alignment in the embedded line edit widget

:param alignment:
%End

void setSpecialValueText( const QString &txt );
%Docstring
Set the special-value text to be ``txt``
If set, the spin box will display this text instead of a numeric value whenever the current value
is equal to minimum(). Typical use is to indicate that this choice has a special (default) meaning.
%End

virtual int valueFromText( const QString &text ) const;
Expand Down
17 changes: 17 additions & 0 deletions src/gui/editorwidgets/qgsdoublespinbox.cpp
View file
Open in desktop
Expand Up @@ -26,6 +26,12 @@

#define CLEAR_ICON_SIZE 16

// This is required because private implementation of
// QAbstractSpinBoxPrivate checks for specialText emptiness
// and skips specialText handling if it's empty
QString QgsDoubleSpinBox::SPECIAL_TEXT_WHEN_EMPTY = QChar( 0x2063 );


QgsDoubleSpinBox::QgsDoubleSpinBox( QWidget *parent )
: QDoubleSpinBox( parent )
{
Expand Down Expand Up @@ -140,13 +146,24 @@ void QgsDoubleSpinBox::setLineEditAlignment( Qt::Alignment alignment )
mLineEdit->setAlignment( alignment );
}

void QgsDoubleSpinBox::setSpecialValueText( const QString &txt )
{
if ( txt.isEmpty() )
QDoubleSpinBox::setSpecialValueText( SPECIAL_TEXT_WHEN_EMPTY );
else
QDoubleSpinBox::setSpecialValueText( txt );
}

QString QgsDoubleSpinBox::stripped( const QString &originalText ) const
{
//adapted from QAbstractSpinBoxPrivate::stripped
//trims whitespace, prefix and suffix from spin box text
QString text = originalText;
if ( specialValueText().isEmpty() || text != specialValueText() )
{
// Strip SPECIAL_TEXT_WHEN_EMPTY
if ( text.contains( SPECIAL_TEXT_WHEN_EMPTY ) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

==?

Copy link
Contributor Author

@elpaso elpaso Sep 18, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No: the problem that is addressed here is that when the widget value is null and the user enters a text, the text gets appended/prepended to the invisible character and the value is not correctly converted to double/int, this is also checked in the tests, so we really want to replace any invisible char in any position in the text.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sidenote: at some point I'll create a number-optimized QLineEdit with proper placeholder text and int/double properties. Noone needs the arrows on spinboxes anyway. And the only thing they reliably get right is data corruption because they grab the mouse wheel.

☠️ death to spinboxes ☠️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and they could even make copy/paste with thousand separators work as expected.

text = text.replace( SPECIAL_TEXT_WHEN_EMPTY, QString() );
int from = 0;
int size = text.size();
bool changed = false;
Expand Down
14 changes: 14 additions & 0 deletions src/gui/editorwidgets/qgsdoublespinbox.h
View file
Open in desktop
Expand Up @@ -132,6 +132,13 @@ class GUI_EXPORT QgsDoubleSpinBox : public QDoubleSpinBox
*/
void setLineEditAlignment( Qt::Alignment alignment );

/**
* Set the special-value text to be \a txt
* If set, the spin box will display this text instead of a numeric value whenever the current value
* is equal to minimum(). Typical use is to indicate that this choice has a special (default) meaning.
*/
void setSpecialValueText( const QString &txt );

double valueFromText( const QString &text ) const override;
QValidator::State validate( QString &input, int &pos ) const override;
void paintEvent( QPaintEvent *e ) override;
Expand All @@ -156,6 +163,13 @@ class GUI_EXPORT QgsDoubleSpinBox : public QDoubleSpinBox
bool mExpressionsEnabled = true;

QString stripped( const QString &originalText ) const;

// This is required because private implementation of
// QAbstractSpinBoxPrivate checks for specialText emptiness
// and skips specialText handling if it's empty
static QString SPECIAL_TEXT_WHEN_EMPTY;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any special reason to put this into the header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the whole variable appears like an internal implementation detail that could only live isolated in the cpp on first look?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, besides that I'm using SPECIAL_TEXT_WHEN_EMPTY in the test, I thought it could be clearer to define a variable for that char instead of hardcoding the utf code directly (if is that what you meant).
But I'm fine to change it if you insist :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't insist, I was just thinking out loud. Mostly I try to keep implementation details out of installed headers.
Do as you wish ;)


friend class TestQgsRangeWidgetWrapper;
};

#endif // QGSDOUBLESPINBOX_H
14 changes: 12 additions & 2 deletions src/gui/editorwidgets/qgsrangewidgetwrapper.cpp
View file
Open in desktop
Expand Up @@ -22,6 +22,8 @@
#include "qgsdial.h"
#include "qgsslider.h"



QgsRangeWidgetWrapper::QgsRangeWidgetWrapper( QgsVectorLayer *vl, int fieldIdx, QWidget *editor, QWidget *parent )
: QgsEditorWidgetWrapper( vl, fieldIdx, editor, parent )

Expand Down Expand Up @@ -119,7 +121,11 @@ void QgsRangeWidgetWrapper::initWidget( QWidget *editor )
// Note: call setMinimum here or setValue won't work
mDoubleSpinBox->setMinimum( minval );
mDoubleSpinBox->setValue( minval );
mDoubleSpinBox->setSpecialValueText( QgsApplication::nullRepresentation() );
QgsDoubleSpinBox *doubleSpinBox( qobject_cast<QgsDoubleSpinBox *>( mDoubleSpinBox ) );
if ( doubleSpinBox )
doubleSpinBox->setSpecialValueText( QgsApplication::nullRepresentation() );
else
mDoubleSpinBox->setSpecialValueText( QgsApplication::nullRepresentation() );
}
mDoubleSpinBox->setMinimum( minval );
mDoubleSpinBox->setMaximum( maxval );
Expand All @@ -141,7 +147,11 @@ void QgsRangeWidgetWrapper::initWidget( QWidget *editor )
int stepval = step.isValid() ? step.toInt() : 1;
minval -= stepval;
mIntSpinBox->setValue( minval );
mIntSpinBox->setSpecialValueText( QgsApplication::nullRepresentation() );
QgsSpinBox *intSpinBox( qobject_cast<QgsSpinBox *>( mIntSpinBox ) );
if ( intSpinBox )
intSpinBox->setSpecialValueText( QgsApplication::nullRepresentation() );
else
mIntSpinBox->setSpecialValueText( QgsApplication::nullRepresentation() );
}
setupIntEditor( minval, max, step, mIntSpinBox, this );
if ( config( QStringLiteral( "Suffix" ) ).isValid() )
Expand Down
17 changes: 17 additions & 0 deletions src/gui/editorwidgets/qgsspinbox.cpp
View file
Open in desktop
Expand Up @@ -26,6 +26,12 @@

#define CLEAR_ICON_SIZE 16

// This is required because private implementation of
// QAbstractSpinBoxPrivate checks for specialText emptiness
// and skips specialText handling if it's empty
QString QgsSpinBox::SPECIAL_TEXT_WHEN_EMPTY = QChar( 0x2063 );


QgsSpinBox::QgsSpinBox( QWidget *parent )
: QSpinBox( parent )
{
Expand Down Expand Up @@ -137,6 +143,14 @@ void QgsSpinBox::setLineEditAlignment( Qt::Alignment alignment )
mLineEdit->setAlignment( alignment );
}

void QgsSpinBox::setSpecialValueText( const QString &txt )
{
if ( txt.isEmpty() )
QSpinBox::setSpecialValueText( SPECIAL_TEXT_WHEN_EMPTY );
else
QSpinBox::setSpecialValueText( txt );
}

int QgsSpinBox::valueFromText( const QString &text ) const
{
if ( !mExpressionsEnabled )
Expand Down Expand Up @@ -185,6 +199,9 @@ QString QgsSpinBox::stripped( const QString &originalText ) const
QString text = originalText;
if ( specialValueText().isEmpty() || text != specialValueText() )
{
// Strip SPECIAL_TEXT_WHEN_EMPTY
if ( text.contains( SPECIAL_TEXT_WHEN_EMPTY ) )
text = text.replace( SPECIAL_TEXT_WHEN_EMPTY, QString() );
int from = 0;
int size = text.size();
bool changed = false;
Expand Down
15 changes: 15 additions & 0 deletions src/gui/editorwidgets/qgsspinbox.h
View file
Open in desktop
Expand Up @@ -132,6 +132,13 @@ class GUI_EXPORT QgsSpinBox : public QSpinBox
*/
void setLineEditAlignment( Qt::Alignment alignment );

/**
* Set the special-value text to be \a txt
* If set, the spin box will display this text instead of a numeric value whenever the current value
* is equal to minimum(). Typical use is to indicate that this choice has a special (default) meaning.
*/
void setSpecialValueText( const QString &txt );

int valueFromText( const QString &text ) const override;
QValidator::State validate( QString &input, int &pos ) const override;

Expand All @@ -156,7 +163,15 @@ class GUI_EXPORT QgsSpinBox : public QSpinBox

bool mExpressionsEnabled = true;

// This is required because private implementation of
// QAbstractSpinBoxPrivate checks for specialText emptiness
// and skips specialText handling if it's empty
static QString SPECIAL_TEXT_WHEN_EMPTY;

QString stripped( const QString &originalText ) const;

friend class TestQgsRangeWidgetWrapper;

};

#endif // QGSSPINBOX_H