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

Jetpacktify our quickstarts ✨ #1217

Closed
12 of 16 tasks
thatfiredev opened this issue Nov 20, 2020 · 13 comments
Closed
12 of 16 tasks

Jetpacktify our quickstarts ✨ #1217

thatfiredev opened this issue Nov 20, 2020 · 13 comments
Assignees
Milestone

Comments

@thatfiredev
Copy link
Member

thatfiredev commented Nov 20, 2020

Following up on my suggestion to implement Modern Android App Architecture here, I've come up with a list of actionable tasks which we can discuss in this issue.

1. Replace RelativeLayout with ConstraintLayout

ConstraintLayout seems to be the recommended way to design layouts in Android, right now. I suggest we replace all our RelativeLayout usages with ConstraintLayout.

Quickstarts to be updated

2. Use the Navigation Component

Starting in Android Studio 4.0, creating a new project now gives you a single activity with 2 fragments and the Navigation Component included to help navigate between the 2 fragments.
I suggest we update our quickstarts to use the same single-activity-multiple-fragments approach with the Navigation Component.

Quickstarts to be updated

3. Use ViewModel+LiveData+Repository

The Android's App Architecture Guide recommends structuring the app with a ViewModel and a Repository for better separation of concerns.
Not only will this be a huge refactor, but we might also run into other issues because of the quickstarts which include snippets that are shown on the Firebase Docs. So I'd suggest we start with the databases quickstarts first, to see if it's worth refactoring the others.

  • Cloud Firestore
  • Realtime Database
@google-oss-bot

This comment has been minimized.

@samtstern
Copy link
Contributor

@rosariopfernandes thanks for starting the conversation and breaking down the tasks!

  • I think (1) is an obvious positive, we should do it.
  • I also think (2) is probably a good idea, I don't think it adds much complexity
  • I am a bit concerned about (3) ... I'll have to see what this looks like in an example before deciding. Quickstarts need to be careful not to "hide" the firebase behind platform abstractions

Oh and don't worry about snippets. I'd be happy to have this excuse to transfer more of them to snippets-android anyway.

@thatfiredev
Copy link
Member Author

@samtstern I share the same concern on (3). I'll see if I can create a small sample app to demonstrate what this would look like.

@samtstern
Copy link
Contributor

@rosariopfernandes I assigned RTDB and Firestore to myself, will start giving them a look today.

@thatfiredev
Copy link
Member Author

@samtstern Thanks for the assists with ConstraintLayout! I'll see if I can finish my draft for auth tonight.

@samtstern
Copy link
Contributor

@rosariopfernandes I'll take the Firestore migration to Navigation so I can get some practice with that.

@thatfiredev
Copy link
Member Author

@samtstern I had a feeling you would take it, that's actually why I kept it for last ;)

@samtstern
Copy link
Contributor

@rosariopfernandes we're so close! I almost feel bad that we'll probably end up deleting the Java at some point in the next year and going Kotlin-only, but I think we'll both be pretty happy about that 😄

@thatfiredev
Copy link
Member Author

@samtstern It'll be a bit sad to see Java go, since we spent so much time maintaining it. But going Kotlin-only will make maintenance much easier, so I'm happy with that. 😄

@thatfiredev
Copy link
Member Author

thatfiredev commented Mar 17, 2021

@samtstern I was taking a look at the firestore quickstart to see how we could implement a ViewModel and I found the FirestoreAdapter implementation very interesting.
But I wonder if we have strong reasons to keep it? Because we could replace it with a custom ListAdapter which computes diff changes for us and since it requires less code than the FirestoreAdapter implementation, it might make it a lot easier for beginners to get a grasp of it.

@samtstern
Copy link
Contributor

@rosariopfernandes hadn't heard of ListAdapter! Everything old is new again. I agree it looks like it could be useful but it also is doing double work by computing the diffs when the Firestore snapshot listener already gives us the diffs between snapshots.

I think since one of the goals of the quickstart is to teach people how to use the Firestore APIs we should probably stick to our current implementation (or something like it) for now.

@samtstern samtstern removed their assignment Jul 16, 2021
@samtstern
Copy link
Contributor

cc @gkaldev who may pick up this work in the future! Kalyan if you plan to do anything related to Firebase + Android + GitHub make sure to say hi to @rosariopfernandes (the hidden wizard behind the curtain).

@thatfiredev
Copy link
Member Author

Superseded by #1457

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

No branches or pull requests

3 participants