Range widget

[tomasMizera]

Complete rework of range widget.

Important parts:

• Range editable and range slider widgets were separated into two qml files
• Range editable is no longer using qml SpinBox many bugs were coming from there. It is now just RowLayout with:
  • minus sign (new icon)
  • number input (text field)
  • optional suffix (text)
  • plus sign (new icon)
• State has been introduced for numeric fields, it is the text underneath field name (the same as constraint description). It informs user when a value he/she typed is either invalid (can not be converted to number with convertCompatible) or is outside of specified min/max range
• We do not use any qml validator, AttributeController serves as validator
• Step is set to have a value that is visible! It is possible to set e.g. precision:3 and step:0.0001 which resulted in number not being incremented (it actually was incremented, but it was not visible). Now step is set as a maximum of step from qgis and minimum visible increment.

Current workflow:

• User types in number or hits +/- sign
• Number is converted to double with Number.fromLocaleString
• Value is sent to AttributeController
• If the value is not valid or the value is outside of the specified range (min/max), state of the formItem is set accordingly
• If any of formItems have not valid state, it is not possible to save the form
• When value is changed, it is converted back to the field with Number.toLocaleString

Also, there was this toast when user inserted some invalid value to a field, something like "Value x is not compatible with field y". It was not working in recent versions, I would just remove it, what do you think?

Needs to be properly tested, best in combinations with default values and apply-on-update values. I have used projects: lutraconsulting/test_decimals, tomas/decimals and tomas/another-decimals.

There are still some edge case bugs and I also need to completely change the tests because in the end, we are not using any validator (AttributeController is our validator). Please add all spotted bugs to comments. Best to test it on mobile device, desktop might handle scope differently (mainly due to a virtual keyboard).

Todo:

• alter and add tests
• revert unused code and tests
• issues to fix:
  • when groups (thousands) separator is used as decimal separator it provides some unexpected results
  • textfield sometimes does not recognize that number is already valid
  • slider with min/max set to +-inf
  • it is possible to save when hitting save directly before commiting text (due to a Qt bug https://bugreports.qt.io/browse/QTBUG-94499?jql=status%20in%20(Reported%2C%20Open)%20AND%20text%20~%20%22qt%20inputmethod%20visible%22%20ORDER%20BY%20created%20DESC)
  • polish loop
  • "invalid value text" disappears if you commit the same text twice
  • field item state remains for other features too
  • would be better to convert text from locale string before user edits it
  • ignore for slider widget
  • handling zero with +/-
• input help documentation
• press and hold on +/- signs

Resolves #1426
Resolves #1435
Resolves #1422
Should fix also #1183 and #493 but need to double check Needs to be fixed with #1507, we are currently not ideally deal null values

Let me know what you think

Screen_Recording_20210618-084502.mp4

[jozefbudac]

Few comments:

• In this PR there is still problem described here: Range slider limit is 29, even when is set to 30 #1497
• as currently all fields are validated for correct value, the question is, if decimal numbers in integer fields should be automatically round the number as it is done right now
• when validation fails for a feature, the field remains marked even when new feature is added
  https://user-images.githubusercontent.com/78033756/122537018-6e6df780-d025-11eb-9e87-8f0642fe2a82.mp4
• when inserting number into the number (as in video) validation fails, even then it should not
  https://user-images.githubusercontent.com/78033756/122537496-e5a38b80-d025-11eb-8576-75435e25f2c8.mp4
• when inserting really big number in field without max value, it is used without warning, +- signs are active, but don't work

[PeterPetrik]

good work!

One top-level question about "constrains [soft/hard]" and "invalid field" etc. In general from QML point of view it is just "string" to show/not show below the field and if user can/cannot save the features. I am wondering if the whole separation of constrains (qgis code concept) and validity (newly introduced input concept) shouldn't be just internal/private implementation detail of attribute controller ....

@wonder-sk @tomasMizera what do you think?

[tomasMizera]

good work!

One top-level question about "constrains [soft/hard]" and "invalid field" etc. In general from QML point of view it is just "string" to show/not show below the field and if user can/cannot save the features. I am wondering if the whole separation of constrains (qgis code concept) and validity (newly introduced input concept) shouldn't be just internal/private implementation detail of attribute controller ....

@wonder-sk @tomasMizera what do you think?

I am 100% for. We could maybe still have two types of constraints:

• soft: those that let's you save the form, e.g. qgis soft constraint or scan an invalid QR code but the field can be null
• hard: those that won't let you save the form, as you said range and qgis hard constraints

@PeterPetrik in this case, would you still keep the formItem's state property or how would we differentiate between different errors?

[PeterPetrik]

maybe just msgText and msgLevel = [None/Info/Warn/Error] and then have a boolean property in attribute controller possible to save or not? not sure

[tomasMizera]

Hm, so AttributeController would set the msgText e.g. number out of range or invalid value?

[tomasMizera]

@PeterPetrik @wonder-sk I have created a separate ticket for the invalid values topic we discussed
#1507

[PeterPetrik]

let me know when it its ready for review, I still see "WIP" in the title?

[tomasMizera]

@PeterPetrik ready for final review, there are some remarks but it is not blocking review

[PeterPetrik]

just 1/2 of the review

[sklencar]

• position of value and suffix (iOS + A)
• still buggy for values that are completely out of the range (too big numbers): +- buttons doesn't work (iOS + A):
• different behaviour as in QGIS: when value is out of range, +- buttons sets value to edge range value (QGIS), Input just in/decreases value by a step. Assuming we want to keep invalid values visible for a user, it is just a note that it works differently.
• not only "invalid value text" disappears, but also (hard) constrain disappears on second click on a field (iOS + A)

[sklencar]

Also slider doesn't work properly (test project: tomas/decimals):

RPReplay_Final1624452537.MP4

However, field config also seems to not be set correctly - range is rather none that set as <-inf, inf>

[sklencar]

Generally besides known issues and small remarks, I can approve it since it works with most of the user cases.
However, best to resolve code comments before.

[tomasMizera]

Also slider doesn't work properly (test project: tomas/decimals):

RPReplay_Final1624452537.MP4
However, field config also seems to not be set correctly - range is rather none that set as <-inf, inf>

Yes, that is just project discrepancy between field name and actual values in range.
There is however one ongoing issue (was here in previous versions too): #1516