Skip to content

Comments

Feedback UI#400

Merged
frederoni merged 15 commits intomasterfrom
fred-feedback-ui
Jul 26, 2017
Merged

Feedback UI#400
frederoni merged 15 commits intomasterfrom
fred-feedback-ui

Conversation

@frederoni
Copy link
Contributor

@frederoni frederoni commented Jul 18, 2017

Feedback UI

@bsudekum @ericrwolfe 👀

@bsudekum
Copy link
Contributor

Would it be possible to split a non UI class out of this that lives in core? This would allow users who use MapboxCoreNavigation to piggy back off the feedback UI class, set appropriate variables and then create their own UI.

@ericrwolfe
Copy link
Contributor

ericrwolfe commented Jul 18, 2017

@bsudekum you can use the following 3 methods on RouteController to hook in feedback events to your own feedback UI:

public func recordFeedback(type: FeedbackType, description: String?) {
enqueueFeedbackEvent(type: type, description: description)
}
/**
Update the last recorded feedback event, for example if you have a custom feedback UI that lets a user elaborate on an issue.
*/
public func updateLastFeedback(type: FeedbackType, description: String?) {
if let lastFeedback = outstandingFeedbackEvents.filter({$0 is FeedbackEvent}).last {
lastFeedback.eventDictionary["feedbackType"] = type.rawValue
lastFeedback.eventDictionary["description"] = description
}
}
/**
Discard the last recorded feedback event, for example if you have a custom feedback UI and the user cancelled feedback.
*/
public func cancelLastFeedback(type: FeedbackType, description: String?) {
if let lastFeedback = outstandingFeedbackEvents.filter({$0 is FeedbackEvent}).last, let index = outstandingFeedbackEvents.index(of: lastFeedback) {
outstandingFeedbackEvents.remove(at: index)
}
}
. Is that what you mean?

@bsudekum
Copy link
Contributor

👍 , @ericrwolfe yeah that looks. I was thinking there was more we could abstract out of here into core but it doesn't look like it.

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.

Holding on illustrations

Copy link
Contributor

Choose a reason for hiding this comment

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

Going to need some docs here.

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 good other than doc addition.

@ericrwolfe ericrwolfe changed the base branch from telem to master July 24, 2017 23:58
@ericrwolfe
Copy link
Contributor

@frederoni added some docs and made feedback events easier to update + cancel. Also cancelled autodismissal on selection and moved dismissal logic to the parent controller so that it's possible to show a confirmation UI afterwards.

@frederoni can you increase the label size as large as possible without wrapping, change the text color to black or near black for added contrast, and change the following label names (keeping at a max of 11 chars)?

  • Turn not allowed ➡️ Not allowed
  • Road closed ➡️ Closure
  • Routing error ➡️ Confusing
  • Other ➡️ Other issue

@willwhite I'm still not sure I feel the route issue choices list is the right balance between clear and simple though comprehensive. Do you think the list covers the majority of issues you've been seeing? Part of me wants to replace Hazard with Closure for now to give space for another route issue option such as Wrong (instruction or route) to complement Confusing. We can bring Hazard back later once we have a better idea of the issues drivers are facing.

@frederoni frederoni merged commit f9c5f73 into master Jul 26, 2017
@frederoni frederoni deleted the fred-feedback-ui branch July 26, 2017 20:14
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