Skip to content

Comments

Inform about weak gps signal#490

Merged
frederoni merged 7 commits intomasterfrom
fred-gps-signal
Aug 15, 2017
Merged

Inform about weak gps signal#490
frederoni merged 7 commits intomasterfrom
fred-gps-signal

Conversation

@frederoni
Copy link
Contributor

@frederoni frederoni commented Aug 14, 2017

Added a new delegate that informs the user about a discarded (non-qualified) location and a corresponding UI. Also simplified the animations by removing all manual constraint animations.

  • Add documentation
  • Fix

@bsudekum 👀

@frederoni frederoni added the ⚠️ DO NOT MERGE PR should not be merged! label Aug 14, 2017
sessionState.pastLocations.push(location)

guard location.isQualified else {
delegate?.routeController?(self, didDiscard: location)
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I like this design!

@bsudekum
Copy link
Contributor

Looks good. My only qualm visually is the spinner- it makes me think we're waiting on something. What do you think about hiding it for this action?

@frederoni
Copy link
Contributor Author

It could continue spinning for as long as we discard locations and then we hide the status view when we get the first qualified location?

@bsudekum
Copy link
Contributor

bsudekum commented Aug 14, 2017

@frederoni I'm suggesting hiding the spinner completely while the gps signal is poor.

Update: retracting my previous comment. This UI element needs to be extensible for future actions that are added to the status view. Stacking might become busy, but let's test it and see how it works.

optional func routeController(_ routeController: RouteController, willRerouteFrom location: CLLocation)

@objc(routeController:didDiscardLocation:)
optional func routeController(_ routeController: RouteController, didDiscard location: CLLocation)
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need some documentation for this new delegate method.

@frederoni frederoni mentioned this pull request Aug 15, 2017
@frederoni frederoni removed the ⚠️ DO NOT MERGE PR should not be merged! label Aug 15, 2017
Copy link
Contributor

@bsudekum bsudekum left a comment

Choose a reason for hiding this comment

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

Looks great

@frederoni frederoni merged commit dd1fca3 into master Aug 15, 2017
@frederoni frederoni deleted the fred-gps-signal branch August 15, 2017 18:54
@bsudekum bsudekum mentioned this pull request Aug 15, 2017
@ericrwolfe ericrwolfe modified the milestone: v0.7.0-3 Aug 16, 2017
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.

3 participants