DroidKaigi / conference-app-2017

The Official Conference App for DroidKaigi 2017 Tokyo
Apache License 2.0
470 stars 140 forks source link

Use injectable Navigator instead of callback of ViewModel #351

Closed k-kagurazaka closed 7 years ago

k-kagurazaka commented 7 years ago

Issue

None

Overview (Required)

This is for suggestion, please close it if you are not a fun of this idea.

The goal of this PR is that remove all callbacks of ViewModel and uses injectable Navigators instead. Currently I completed following tasks:

I want to get reviews before working remaining tasks, so please don't merge this PR now.

konifar commented 7 years ago

Thanks! WebNavigator seems good. I have a question! 🙋 Do you have any idea other Navigator class for Activity/Dialog?

k-kagurazaka commented 7 years ago

I have two plans:

  1. Separate Navigator for each view, like InformationViewNavigator.

Pros.

Cons.

  1. Create ViewNavigator that will be used for all transition.

Pros. and Cons. are reverse of plan 1.

Now I think plan 2 is better, what do you think about?

konifar commented 7 years ago

@k-kagurazaka Thanks! I also think 2 is better since this app doesn't have so many views 😃

k-kagurazaka commented 7 years ago

@konifar Thanks for your quick reply. I will try to implement ViewNavigator.

k-kagurazaka commented 7 years ago

I rewrite activity transition by injectable Navigator. Currently some Callback interfaces exist yet, however we should apply MVVM pattern before removing these callbacks. So I changed the focus of PR as changed title.

Would you like to review?

k-kagurazaka commented 7 years ago

I just extract Navigator from BaseActivity 😄

konifar commented 7 years ago

LGTM! :cool: Thanks for great Refactoring! I learned a lot from this