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

[Predictive Back] Add a new Predictive Back Samples folder. #94

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

ashnohe
Copy link
Contributor

@ashnohe ashnohe commented Sep 9, 2023

Set up recycler view to house interactive cards that will eventually show multiple samples. The recycler view does not currently have interactive samples. For now it describes how to view back-to-home and cross-activity animations.

https://photos.app.goo.gl/QGL7DrBtMbGVqTLk7

Set up recycler view to house interactive cards that will eventually show multiple samples. The recycler view does not currently have interactive samples. For now it describes how to view back-to-home and cross-activity animations.
@ashnohe ashnohe requested a review from marcelpinto September 9, 2023 00:00
Copy link
Contributor

@marcelpinto marcelpinto left a comment

Choose a reason for hiding this comment

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

Are you planning to showcase it with compose too?


package com.example.platform.ui.predictiveback

class PBAnimation(val title: String, val description: String)
Copy link
Contributor

Choose a reason for hiding this comment

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

make it data class for just class holders

Copy link
Contributor Author

Choose a reason for hiding this comment

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

acknowledged


class PBAnimation(val title: String, val description: String)

val pbAnimations = mutableListOf<PBAnimation>().apply {
Copy link
Contributor

Choose a reason for hiding this comment

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

it's better to use the constructor directly

mutableListOf(PBAnimation(...), PBAnimation(..), ...)

And if this list is not modified then defnie the variable type to be the immutable version List

Copy link
Contributor Author

Choose a reason for hiding this comment

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

acknowledged


import androidx.lifecycle.ViewModel

class PBViewModel: ViewModel() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for this I would skip the VM. Just use the variable directly. It's simple and clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

acknowledged

name="Predictive Back Sample",
description="Shows Predictive Back animations."
)
class PBHostingActivity : AppCompatActivity() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the activity needed to demostrate the sample? If not maybe you could just use the fragment directly. This way you can skip the theme and all the boilerplate that comes with activity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the activity is needed. From the sample page folks can swipe back to see the cross-activity animation. I'm also hoping to add a custom cross-activity animation.

@ashnohe
Copy link
Contributor Author

ashnohe commented Sep 11, 2023

Compose side of things tbd

@ashnohe ashnohe requested a review from marcelpinto September 11, 2023 19:47
@ashnohe ashnohe merged commit 71ee1d4 into main Sep 18, 2023
4 checks passed
@ashnohe ashnohe deleted the predictive_back2 branch September 19, 2023 21:51
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