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.

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

Accuracy warning while recording a feature #1221

Merged
merged 6 commits into from
Mar 19, 2021
Merged

Conversation

sklencar
Copy link
Contributor

@sklencar sklencar commented Mar 3, 2021

  • should be visible while recording a new feature or edit geometry of existing feature
  • should be visible if accuracy is none or bellow limit set in settings
  • if there is no gps position, only a toast will be shown as before
  • It seems that position kit remembers last position. Therefore in the case when gps services are turned of, the gps warning banner will be shown instead of the toast.
  • shown GPS accuracy - precision 1 decimal

Banner has still simple design with suggested colors. Would be good to ask Rado for a desing.
This PR also stressed #1225 even more.

closes #1192
Screenshot 2021-03-09 at 15 35 56

@sklencar sklencar requested a review from wonder-sk March 3, 2021 17:06
@saberraz
Copy link
Contributor

saberraz commented Mar 4, 2021

Notes:

  • The warning will appear only if GPS is used for capturing points (i.e. when user moves the map the warning will disappear)
  • Display the warning when using "streaming"
  • A setting option (below Accuracy threshold) to disable the warning (warning will be on by default)
  • Link to documentation for what these stuff mean

Copy link
Collaborator

@tomasMizera tomasMizera left a comment

Choose a reason for hiding this comment

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

The banner looks and works very well 🚀

One more note:

  • Turn off location service, you will get 2 notifications about no GPS (banner and toast) - this could be handled differently

  • Moreover, it shows -1m in that case ^^

app/qml/main.qml Outdated Show resolved Hide resolved
app/qml/SettingsPanel.qml Outdated Show resolved Hide resolved
@sklencar
Copy link
Contributor Author

sklencar commented Mar 9, 2021

@saberraz For time-being, the link in the warning takes a user to https://help.inputapp.io/. Is there any more specific link we want to have it there instead?

@saberraz
Copy link
Contributor

saberraz commented Mar 9, 2021

@sklencar could you point it to https://help.inputapp.io/howto/gps_accuracy

@wonder-sk
Copy link
Contributor

I would suggest few more changes:

  • move the warning to the top of the screen (as discussed in the last call)
  • add some light shadow around the whole rectangle so that it stands out from the map a bit
  • use black foreground color and some brigher yellow (let's try e.g. #ffff99) - the green-on-orange looks a bit dull to me
  • the two different sizes of text look a bit strange to me - probably better use just a single font size
  • Instead of "More information [here]" it would be good to have "[Learn more]" link, probably preceded with a sentence giving a bit of clue... e.g. "Please make sure you have good view of the sky. [Learn more]" -- I'm sure PeteW can then come up with a better copy text :-)

Copy link
Collaborator

@tomasMizera tomasMizera left a comment

Choose a reason for hiding this comment

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

Great job, it is working very well!

I would maybe change the text so that it is clear why we ask user to have a nice view of the sky (there is now nothing written about GPS accuracy), but for sure @mostlyAtNight will come up with something :)
Other than that, I am ok to merge!

@wonder-sk
Copy link
Contributor

@sklencar ah yes can we please keep also the first sentence (Low GPS position accuracy (XXX m)) - and below that let's have the second sentence about the view of the sky - sorry if I was unclear with my comments.

Looks like there are also some conflicts now from the other merges - could you please rebase?

@sklencar
Copy link
Contributor Author

Added additional text to banner

@wonder-sk wonder-sk merged commit d73403c into master Mar 19, 2021
@PeterPetrik PeterPetrik deleted the 1192_accuracy_warning branch April 15, 2021 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show a warning when the accuracy is low
5 participants