Open mitxel-m opened 11 months ago
The PR does not take the differences between imperial and nautical units into account.
Internally QMS uses and stores metric values. When these values are passed to the UI or read from the UI you need to use one of the methods defined by the instance you get by calling IUnit::self(). This will also provide you with the correct unit string to be displayed.
Switching between the metric, imperial and nautical system must be a coherent user experience.
Ok, I will modify it. The first proposal focused on making as few changes as possible in terms of usage. That's why it only rounded distances for Metric and Imperial units, and kept them unrounded for Nautic and Aviation, exactly as is currently done in version 1.17.1
But I agree that to be coherent once the user gets the option to round distances, it should be applicable whatever units type he/she chooses.
So I'm planning to do :
would this meet the requirements?
Apart from that, and thanks to @kiozen 's previous comment, I have realised that this same thing is wrong in the UI of the speed filter for the tracks. It doesn't show the correct unit suffix, but always shows km/h even if you use mi/h, which leads to confusion.
I think I will be able to fix it, but I would like to do it in a different issue once we finish this one.
I think it is now ready to review with commit [1634499]
I hope I find some time to work on this soon. Storing 20000m as 20 and applying all these hardcoded factors is hell to maintain. Imho the IUnit API should provide enough methods to handle this properly. However I can not sketch a simple solution for that. Therefore I need to find the time to create a patch.
I still doubt that this does make much sense with given resolution and precision of maps and GPS devices.
What is the linked issue for this pull request:
QMS-#651
What you have done:
Add a box in the units configuration that allows the user to set from which value the rounding of distances will be done for metric and imperial units.
It is not necessary that this value exists previously in the configuration file, if it does not exist it is set the first time we use the units dialog. This gives compatibility with existing conf files.
It respects the current concept (which I like) that the larger the distance the less decimal digits are displayed.
It is very simple to use. By default we have the value 20 which corresponds to the current value and gives us this:
If you set the value to 80 , it would use these ranges
and so on ...
Steps to perform a simple smoke test:
For convenience you can download this gpx file containing 5 tracks, named with different distances. Open it in QMS and go to edit the project, so that you see the summary with the data of all 5 tracks in the same view.
First check that you see the distances with the decimals according to the current behaviour.
Try with another value: 'Menu - View - Setup Units' Modify the value. You can use the mouse wheel. Then click on refresh (top right) on the project summary view and check how you see now the distances according to the value you have set. You can repeat it for different values.
Check how the distances are displayed with the ruler.
You can also change the unit to miles (Imperial) and see how it works.
Does the code comply to the coding rules and naming conventions Coding Guidelines:
Is every user facing string in a tr() macro?
Did you add the ticket number and title into the changelog? Keep the numeric order in each release block.
Remark: My C++ skills are very limited and I have been able to write this by looking at other parts of the code. Probably my proposal can be refined.