[bugfix] Allow empty null representation in spinboxes #7943
Conversation
Fixes qgis#18583 - Widget "Range": Representation for NULL value is NOT always displayed correctly
| 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 ) ) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
👍
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 ☠️
There was a problem hiding this comment.
Oh, and they could even make copy/paste with thousand separators work as expected.
| // 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; |
There was a problem hiding this comment.
Any special reason to put this into the header?
There was a problem hiding this comment.
Do you mean the comment?
There was a problem hiding this comment.
No, the whole variable appears like an internal implementation detail that could only live isolated in the cpp on first look?
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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 ;)
|
Off topic: does the |
Not that I'm aware of, but it goes into git logs while a tag doesn't. Do you suggest we don't use it? I was asking myself the same question. |
|
I'm suggesting to not use it for as long as I don't see a reason why we use it. Possible reasons (but no idea if they apply)
At the same time, I started to put tags that are only for webhooks in less prominent places. The headline for a commit is short enough to be meaningful already. What I would propose: |
Fixes #18583 - Widget "Range": Representation for NULL value is NOT always displayed correctly