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

Deep linking #95

Closed
kakai248 opened this Issue Jun 25, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@kakai248

kakai248 commented Jun 25, 2018

Hello,

I'm currently assessing different navigation libraries to a new app. One of the requirements is deep linking. I know that AAC navigation solves this, but brings a whole lot of new issues and limitations that I'm not comfortable with.

Your library seems to offer a very good control of the backstack, but since I never used it, I need to ask you: Would it be possible to add external deep linking to it? As in, implemented by me on top of the library? Is the API surface enough to support this?

Thank you

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jun 25, 2018

Owner

Hello!

Technically the way I did that was that I could create a pending intent in a notification which would contain a history to set as a target, which was ArrayList<Parcelable> (because my keys are generally parcelable as per default behavior).

If the notification starts the Activity as a new task (instead of singleTop with onNewIntent), then you can provide something like MainActivity.startWith(someKey, anotherKey) and have

backstackDelegate.onCreate(savedInstanceState, lastCustomNonConfigurationInstance, 
    intent?.getParcelableArrayListExtra("targetHistory")?.let { History.from(it) } ?: History.of(initialKey))

Then it should get you what you want, very similarly to this example.

If you are using onNewIntent, it's a similar mechanism except you'll probably want to use backstack.setHistory(History.from(...), StateChange.REPLACE)

In this sense it's kinda "DIY" but it shouldn't be that hard. I was previously debating adding something like the intent integration in Flow but the API for it was tricky so I didn't feel like copying it.

At least one of those should hopefully do as you need, although if you run into quirkiness then I can try to mirror what Nav AAC does in this regard. I know they have actions but I hadn't looked into it further than that yet.

Owner

Zhuinden commented Jun 25, 2018

Hello!

Technically the way I did that was that I could create a pending intent in a notification which would contain a history to set as a target, which was ArrayList<Parcelable> (because my keys are generally parcelable as per default behavior).

If the notification starts the Activity as a new task (instead of singleTop with onNewIntent), then you can provide something like MainActivity.startWith(someKey, anotherKey) and have

backstackDelegate.onCreate(savedInstanceState, lastCustomNonConfigurationInstance, 
    intent?.getParcelableArrayListExtra("targetHistory")?.let { History.from(it) } ?: History.of(initialKey))

Then it should get you what you want, very similarly to this example.

If you are using onNewIntent, it's a similar mechanism except you'll probably want to use backstack.setHistory(History.from(...), StateChange.REPLACE)

In this sense it's kinda "DIY" but it shouldn't be that hard. I was previously debating adding something like the intent integration in Flow but the API for it was tricky so I didn't feel like copying it.

At least one of those should hopefully do as you need, although if you run into quirkiness then I can try to mirror what Nav AAC does in this regard. I know they have actions but I hadn't looked into it further than that yet.

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jul 14, 2018

Owner

@kakai248 did you succeed?

Owner

Zhuinden commented Jul 14, 2018

@kakai248 did you succeed?

@kakai248

This comment has been minimized.

Show comment
Hide comment
@kakai248

kakai248 Jul 19, 2018

Hey,

Sorry for the delay. I completely forgot.

We managed to build a routing class that has a list of routes and then when given an url, it generates a list of screens (what you call key).

router = Router({ HomeScreen() },
                Route("home") { HomeScreen() },
                Route("dashboard") { DashboardScreen() },
                Route("dashboard/one") { OneScreen() },
                Route("dashboard/one/two") { TwoScreen() },
                Route("dashboard/one/three/{id}/{string}") { ThreeScreen(it) },
                Route("dashboard/one/three/{id}/{string}/two") { TwoScreen() }
        )

It's defined this way (it could use a DSL). The first lambda is the root. It can pass parameters through a StateBundle that each screen receives (the it you see).

A test to show how to call it:

router.routeFor("dashboard/one/two")
                .map { it::class }
                .equals(listOf(HomeScreen::class, DashboardScreen::class, OneScreen::class, TwoScreen::class))
                .assert()

Your library is pretty nice, I really like the control it gives (we're using it with fragments). At the first the naming of the classes is not obvious, but you get used to it (we changed key to screen as we think it made more sense).

The way we built the navigation for this app was based on nested fragments. We have a single activity with fragments, but those fragments themselves can have a backstack. This seems to be working fine, the only thing we had to "ignore" was passing a null as the nonConfigurationInstance as the fragment didn't have one. Will we have any problem with this?

backstackDelegate.onCreate(savedInstanceState,
                null,
                History.from(handleDeepLink(screen, true, false))
        )

We like this approach (even if nested fragments are somehow not recommended) because we can separate the screens in flows. For example, we have a LoginFragment (that has backstack) that has a progress bar at the top and replaces it's screens depending on the state.

Other thing I found is that the backstack requires an initial state. Would it be possible to be optional? This way we could observe LiveData and initialize the proper screen depending on the current view state that is stored on the ViewModel.

Thanks!

kakai248 commented Jul 19, 2018

Hey,

Sorry for the delay. I completely forgot.

We managed to build a routing class that has a list of routes and then when given an url, it generates a list of screens (what you call key).

router = Router({ HomeScreen() },
                Route("home") { HomeScreen() },
                Route("dashboard") { DashboardScreen() },
                Route("dashboard/one") { OneScreen() },
                Route("dashboard/one/two") { TwoScreen() },
                Route("dashboard/one/three/{id}/{string}") { ThreeScreen(it) },
                Route("dashboard/one/three/{id}/{string}/two") { TwoScreen() }
        )

It's defined this way (it could use a DSL). The first lambda is the root. It can pass parameters through a StateBundle that each screen receives (the it you see).

A test to show how to call it:

router.routeFor("dashboard/one/two")
                .map { it::class }
                .equals(listOf(HomeScreen::class, DashboardScreen::class, OneScreen::class, TwoScreen::class))
                .assert()

Your library is pretty nice, I really like the control it gives (we're using it with fragments). At the first the naming of the classes is not obvious, but you get used to it (we changed key to screen as we think it made more sense).

The way we built the navigation for this app was based on nested fragments. We have a single activity with fragments, but those fragments themselves can have a backstack. This seems to be working fine, the only thing we had to "ignore" was passing a null as the nonConfigurationInstance as the fragment didn't have one. Will we have any problem with this?

backstackDelegate.onCreate(savedInstanceState,
                null,
                History.from(handleDeepLink(screen, true, false))
        )

We like this approach (even if nested fragments are somehow not recommended) because we can separate the screens in flows. For example, we have a LoginFragment (that has backstack) that has a progress bar at the top and replaces it's screens depending on the state.

Other thing I found is that the backstack requires an initial state. Would it be possible to be optional? This way we could observe LiveData and initialize the proper screen depending on the current view state that is stored on the ViewModel.

Thanks!

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jul 24, 2018

Owner

@kakai248 I really like that Router! So cool!

Your library is pretty nice, I really like the control it gives (we're using it with fragments).

Glad you think so too! IMO it is amazing especially compared to the original addToBackStack stuff -- your fragments are always in the state you expect them to be!

At the first the naming of the classes is not obvious, but you get used to it (we changed key to screen as we think it made more sense).

Even Flow originally called them Screens, then they named them Keys, so I inherited their new name -- but I did see that people tend to get confused by "wait, Key? what?" while "Screen" seems intuitive.

This seems to be working fine, the only thing we had to "ignore" was passing a null as the nonConfigurationInstance as the fragment didn't have one. Will we have any problem with this?

The non-config instance stuff matters only when you have enqueued state change(s?) between onPause and the next onCreate, because it would be enqueued, onDestroy force-executes the last one (if it was already on-going), and onCreate would initialize with the new state. If the backstack is recreated from saved state, then state changes enqueued after onPause would be lost.

I think in most common cases, there aren't enqueued state changes after onPause, so it's OK.

(even if nested fragments are somehow not recommended)

I'm happy you guys made it work 👍

Other thing I found is that the backstack requires an initial state. Would it be possible to be optional?

Technically if you call setHistory before calling setStateChanger then you can initialize it to wherever you want. I don't know if that suits your needs.

Owner

Zhuinden commented Jul 24, 2018

@kakai248 I really like that Router! So cool!

Your library is pretty nice, I really like the control it gives (we're using it with fragments).

Glad you think so too! IMO it is amazing especially compared to the original addToBackStack stuff -- your fragments are always in the state you expect them to be!

At the first the naming of the classes is not obvious, but you get used to it (we changed key to screen as we think it made more sense).

Even Flow originally called them Screens, then they named them Keys, so I inherited their new name -- but I did see that people tend to get confused by "wait, Key? what?" while "Screen" seems intuitive.

This seems to be working fine, the only thing we had to "ignore" was passing a null as the nonConfigurationInstance as the fragment didn't have one. Will we have any problem with this?

The non-config instance stuff matters only when you have enqueued state change(s?) between onPause and the next onCreate, because it would be enqueued, onDestroy force-executes the last one (if it was already on-going), and onCreate would initialize with the new state. If the backstack is recreated from saved state, then state changes enqueued after onPause would be lost.

I think in most common cases, there aren't enqueued state changes after onPause, so it's OK.

(even if nested fragments are somehow not recommended)

I'm happy you guys made it work 👍

Other thing I found is that the backstack requires an initial state. Would it be possible to be optional?

Technically if you call setHistory before calling setStateChanger then you can initialize it to wherever you want. I don't know if that suits your needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment