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

Migrate in-app gallery to Jetpack Compose #468

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

MHShetty
Copy link
Member

No description provided.

@MHShetty MHShetty force-pushed the compose_gallery branch 3 times, most recently from 3bf2d2b to 24a686b Compare September 1, 2024 09:44
@MHShetty MHShetty force-pushed the compose_gallery branch 4 times, most recently from 17b5b55 to 3d31d6b Compare October 13, 2024 21:03
@muhomorr
Copy link
Member

muhomorr commented Nov 8, 2024

What's the purpose of new (de)serializers for CapturedItem and Uri? They can be (de)serialized already since they implement the Parcelable interface.

@MHShetty
Copy link
Member Author

MHShetty commented Nov 8, 2024

Both the Kotlin Serializable and the new Parcelable implementations are needed for using type-safe navigation/custom objects in nav types. Initially, we were passing CapturedItem and List of CapturedItem (s) to specific routes/screen as parameters, but we later switched to having an activity-level repository to manage the captured items which is a much better and cleaner approach.

The only serializer and parcelable implementation we are currently using in the app is VideoUriSerializer class and the Parcelable implemented within the VideoUri class itself. I have kept the other classes just in case if we need them later within the camera app or the gallery app. Type-safe navigation isn't very well documented unfortunately, a lot of the limitations it has isn't very clearly mentioned, so having the other serializable and nav type classes as reference would be helpful for future use

@muhomorr
Copy link
Member

muhomorr commented Nov 9, 2024

What I'm saying is that Kotlin (de)serializers can reuse the implementation of Parcelable (de)serializers instead of duplicating them.

@MHShetty
Copy link
Member Author

I'm sorry but am really unable to get you, serializer/serialization and parcelable are two different concepts/techniques. Is there some library to encode/decode a parcelable to/from a string or work directly with parcelables?

@muhomorr
Copy link
Member

Parcelization is a form of serialization. Any Parcelable object can be transformed into a sequence of bytes by writing it to a Parcel and then calling Parcel#marshall().

@muhomorr
Copy link
Member

A better solution in this case would probably be to not use route objects at all, since they don't work well with Parcelables. There's an option to pass route arguments as a Bundle instead. Bundle supports Parcelables directly.

@MHShetty
Copy link
Member Author

Route objects help us to do type-safe navigation by ensuring the right parameters are called (with respect to their type, order and in avoiding null check/assertions), and also improving the code readability while performing navigate calls apart from the compile-time checks it provides. Since we have already written the code for the same, I feel it's better to go ahead with the standard type-safe navigation approach

@t895 t895 mentioned this pull request Nov 26, 2024
@MHShetty MHShetty force-pushed the compose_gallery branch 5 times, most recently from 124a21c to c9b9389 Compare November 28, 2024 09:13
@MHShetty MHShetty marked this pull request as ready for review November 29, 2024 23:06
(currently causes unexpected UI behavior; can be dealt with while migrating the rest of the camera app to Compose)
(when clicking on an image in extended gallery)
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