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

Navigating to activities using starter pattern #51

Merged
merged 3 commits into from
Dec 3, 2020

Conversation

sidhu18
Copy link
Contributor

@sidhu18 sidhu18 commented Dec 3, 2020

- Summary

Move the activity transaction logic to the target activity using the starter pattern. This pattern really helps when we are working in collaborative projects. When a change is made, say a new extra parameter is required for an activity, this wouldn't throw a compile-time error, but our app will crash during runtime.

With this, we can catch during compilation, and project contributors can know the required params for starting the activity.

More information on this pattern: https://blog.mindorks.com/learn-to-write-good-code-in-android-starter-pattern

- Description for the changelog

Instead of starting activities by creating Intent in the calling method, we call a function which is written in the Activity to start the activity.

Existing code:


  val intent = Intent(this, PostDetailsActivity::class.java)
        intent.putExtra(PostDetailsActivity.POST_ID, post.id)

  val options = ActivityOptionsCompat.makeSceneTransitionAnimation(
        this,
        imageView,
        imageView.transitionName
   )

 startActivity(intent, options.toBundle())

Updated:

 val options = ActivityOptionsCompat.makeSceneTransitionAnimation(
       this,
       imageView,
       imageView.transitionName
 )

 PostDetailsActivity.start(this, post.id, options)

- A picture or screenshot regarding change (not mandatory but encouraged)

@sidhu18 sidhu18 requested a review from PatilShreyas as a code owner December 3, 2020 06:09
Copy link
Owner

@PatilShreyas PatilShreyas left a comment

Choose a reason for hiding this comment

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

@sidhu18 Thanks for this PR 😃. I really liked that approach and thanks for the blog link too. Here are few comments then I'll be happy to merge this.

Comment on lines 117 to 121
fun start(context: Context, postId: Int?, options: ActivityOptionsCompat) {
val intent = Intent(context, PostDetailsActivity::class.java)
intent.putExtra(POST_ID, postId)
context.startActivity(intent, options.toBundle())
}
Copy link
Owner

Choose a reason for hiding this comment

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

This function shouldn't start activity internally using Context of the other Activity. I referred that blog link which you shared. Means we should be confident who's starting the activity. So it would be great if we start the activity from MainActivity itself.

I would suggest to make this method like... 👇🏻

fun getStartIntent(context: Context, postId: Int) = Intent(context, PostDetailsActivity::class.java).apply {
    putExtra(POST_ID, postId)
}

From MainActivity, let's do like... 👇🏻

val intent = PostDetailsActivity.getStartIntent(this, post.id)
startActivity(intent, options.toBundle())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. Thanks for the quick review and clarification. I will make the necessary changes and push it. 😄

Copy link
Owner

@PatilShreyas PatilShreyas left a comment

Choose a reason for hiding this comment

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

Thanks, @sidhu18! 😄. LGTM, happy to merge it 🚀. Feel free to contribute again.

Comment on lines +116 to +118
fun getStartIntent(context: Context, postId: Int?) = Intent(context, PostDetailsActivity::class.java).apply {
putExtra(POST_ID, postId)
}
Copy link
Owner

Choose a reason for hiding this comment

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

Now looks perfect 💯

Comment on lines +191 to 192
val intent = PostDetailsActivity.getStartIntent(this, post.id)
startActivity(intent, options.toBundle())
Copy link
Owner

Choose a reason for hiding this comment

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

Great! 😃

@PatilShreyas PatilShreyas merged commit 05438f8 into PatilShreyas:master Dec 3, 2020
@PatilShreyas
Copy link
Owner

@sidhu18 You can also contribute to this repository with same change if sounds good to you: https://github.com/PatilShreyas/NotyKT/

@sidhu18
Copy link
Contributor Author

sidhu18 commented Dec 3, 2020

@sidhu18 You can also contribute to this repository with same change if sounds good to you: https://github.com/PatilShreyas/NotyKT/

That sounds great, I will definitely look into that. 😀

@sidhu18 sidhu18 deleted the opt-starter-pattern branch December 3, 2020 10:13
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.

2 participants