mapbox / mapbox-navigation-android

Mapbox Navigation SDK for Android
https://docs.mapbox.com/android/navigation/overview/
Other
622 stars 319 forks source link

FeedbackDetailedFragment and FeedbackArrivalFragment force user uses #3852

Closed RingerJK closed 3 years ago

RingerJK commented 3 years ago

Problem

FeedbackFlowListener is passed as an argument directly to FeedbackDetailedFragment and FeedbackArrivalFragment which is mean it cannot be stored in view(screen rotation kicks listener). Implicitly we force users to store listener outside of view(like into ViewModel or smt like this).

https://github.com/mapbox/mapbox-navigation-android/blob/7f56d62164c226b1da499f99972a0e0c5c168030/libnavigation-ui/src/main/java/com/mapbox/navigation/ui/internal/feedback/FeedbackDetailsFragment.kt#L495-L510

Possible solution

Using ViewModel is a common approach and still might be useful to store listener into it for users. So we could keep current approach and add a few more getting listener from activity/fragment as an expecting interface which they have to implement:

class FeedbackDetailsFragment : DialogFragment() {
  fun callFeedbackListener() {
    when {
      parentFragment is FeedbackFlowListener -> {
       parentFrament.callListener()
      }
      activity is FeedbackFlowListener -> {
         activity.callListener()
      }
      parentFragment is FeedbackFlowListenerHolder -> {
         parentFrament.getFeedbackFlowListener().callListener()
      }
      activity is FeedbackFlowListenerHolder -> {
         activity.getFeedbackFlowListener().callListener()
      }
      else -> throw IllegalStateException("Parent activity/fragment has to implement FeedbackFlowListener or FeedbackFlowListenerHolder")    
  }

  interface FeedbackFlowListenerHolder {
    fun getFeedbackFlowListener(): FeedbackFlowListener
  }
}

/cc @LukasPaczos @Guardiola31337

truburt commented 3 years ago

This is not applicable to v2.0 anymore and will be covered with new Feedback UI in one of the subsequent releases.