feat: deep links (android only)

[tom-anders]

No description provided.

[veloce]

Is an external plugin required to support deep links? I think not, according to flutter documentation. It would be better to do without imo.

[tom-anders]

Hmm I'm not sure, the flutter docs recommend this package as an alternative to using go_router (see note here https://docs.flutter.dev/cookbook/navigation/set-up-app-links#2-modify-androidmanifest-xml)

I'll check if there's a way to get the URI Stream without a plugin

[veloce]

The only mention of app_links I see in the doc is this:

If you use a third-party plugin to handle deep links, such as app_links, Flutter's default deeplink handler will break these plugins.

The router and deep links are 2 different things. They recommend to use a router package to handle deep links because deep links can be associated with complex routing scenarios, and for this you need a router.

But we can certainly handle deep links in a simple, straightforward way. So the router is not at all needed. We only need to parse the urls of the links to be able to decide what to do (push the corresponding screen).

[tom-anders]

Yeah I thought app_links is needed to actually get access to the link URL - haven't checked yet if there is a native way do get it, there probably is, I'll have a look

[tom-anders]

Right, found the way to do it without dependencies.

I only added the implementation for Android (as I'm neither familiar with Swift/iOS, nor do I have the hardware to actually test this)

[julien4215]

Nice work ! I made some small comments on the code.

One small thing I noticed when testing it is that you can see the home screen before the one requested by the app link. Also when you press the back button you always return to the home even if you opened a study via a app link. Maybe that can be avoided by listening to the provider higher in the widget tree but it will make the code more complex.

[julien4215]

I tried to use the Flutter's default deeplink handler here.

The advantage is that there is no need to write native source code and using Flutter method channels as it is already done by the default Flutter's default deeplink handler. I couldn't find any documentation on how it works, but from what I saw in the source code it does the same as this PR. The route information is pushed in this platform channel NavigationChannel.

The problem is that I am currently pushing only one route when a app link is used so for example if a user clicks on a puzzle link there isn't the back button to go back to puzzle tab screen as only the puzzle screen route was pushed. Maybe using the go router package would make it easier to push and pop the correct screens when dealing with app links.

[tom-anders]

I tried to use the Flutter's default deeplink handler here.

The advantage is that there is no need to write native source code and using Flutter method channels as it is already done by the default Flutter's default deeplink handler. I couldn't find any documentation on how it works, but from what I saw in the source code it does the same as this PR. The route information is pushed in this platform channel NavigationChannel.

Ah very cool! Yeah documentation is a bit lacking there, I thought you could only use the default link handler if you also use the go_router package, but apparently not! Does it also work for a cold start? i.e. force close the app, and then open it directly via a link?

The problem is that I am currently pushing only one route when a app link is used so for example if a user clicks on a puzzle link there isn't the back button to go back to puzzle tab screen as only the puzzle screen route was pushed. Maybe using the go router package would make it easier to push and pop the correct screens when dealing with app links.

@veloce Was against using go_router here - Maybe we can just change to the correct tab and only then push the new screen?

[tom-anders]

Nice work ! I made some small comments on the code.

One small thing I noticed when testing it is that you can see the home screen before the one requested by the app link. Also when you press the back button you always return to the home even if you opened a study via a app link. Maybe that can be avoided by listening to the provider higher in the widget tree but it will make the code more complex.

Thanks for the review :)

[julien4215]

Yes the default link handler also works with a cold start.

According to the documentation there are some limitations to use only the Navigator described here. From what I understand we will not be able to push multiple screens or change the tab with app links, at least without using some hacks. So for instance, if a user presses an app link that leads to a broadcast game, when he will press the back button he will be brought back to the home tab screen instead of the broadcast round screen.

[veloce]

For the record, when I was prototyping the app, I decided against using go_router for 2 reasons:

1. the package was not really ready at the time, still missing a key feature, state persistence across tabs
2. I though it would add much more unnecessary complexity for unclear added value

Now, even with the deep link I think we still should not use go_router. First it would be a very big change, and second I think the limitations are not a deal breaker in our case. If you want to support web, of course then it is mandatory. But for an app, I like the simple straightforward approach of a stack based navigation.

Choosing this approach we have to accept the limitations, and not adding hack to work around them. I think it is fine to push a screen from a deep link on top of the wrong tab. And when you go back, you just go back to where you were. I can be weird in some scenarios, maybe, as I cannot foresee every corner case or usage. But for the most part it will be enough. It will also force us to keep the deep link feature simple (ie: not handling too much cases), which is a good thing in my opinion.

[veloce]

I tried to use the Flutter's default deeplink handler here.

The advantage is that there is no need to write native source code and using Flutter method channels as it is already done by the default Flutter's default deeplink handler. I couldn't find any documentation on how it works, but from what I saw in the source code it does the same as this PR. The route information is pushed in this platform channel NavigationChannel.

Thanks for trying this. I think we definitely should use the default flutter link handler.

The problem is that I am currently pushing only one route when a app link is used so for example if a user clicks on a puzzle link there isn't the back button to go back to puzzle tab screen as only the puzzle screen route was pushed. Maybe using the go router package would make it easier to push and pop the correct screens when dealing with app links.

What does it mean? If we push the puzzle screen from a deep link there is no back button displayed on the screen? If that is the case it is indeed an issue. We should understand why it is the case and make sure we can go back from any screen pushed from a deep link. Using go_router to solve this problem sounds overkill, and I explained in another comment why I don't think we should use go_router.

[julien4215]

Thanks for explaining why you don't want to go with go_router. I would also like to keep things simple but I have too say that was not easy to find how to use the flutter default deeplink handler. The Flutter documentation recommends using the router for most apps and they explain really little about how you should without it. Also the fact that we don't have only static routes but some with ids made it harder like /study/<id>.

What does it mean? If we push the puzzle screen from a deep link there is no back button displayed on the screen? If that is the case it is indeed an issue. We should understand why it is the case and make sure we can go back from any screen pushed from a deep link. Using go_router to solve this problem sounds overkill, and I explained in another comment why I don't think we should use go_router.

Yes you understood the problem correctly but it was only happening with a cold start of the app using a deeplink. I was able to fix the problem by using onGenerateInitialRoutes.

[tom-anders]

@julien4215 Do we want to open a new PR with your version? I have no problem with you taking over the work :)

[julien4215]

I don't know, I did not really intended to work on deep links. I just looked at your PR because I was interested to see how it works and well I did a bit more than I intended. Feel free to take over my work too if you want.

[tom-anders]

I don't know, I did not really intended to work on deep links. I just looked at your PR because I was interested to see how it works and well I did a bit more than I intended. Feel free to take over my work too if you want.

That's fine as well, I just wanted to avoid duplicate work 😅

[julien4215]

Go ahead then ! I'll focus on bookmark.

[tom-anders]

Ok, changed the implementation to user Flutter's native deep link handling, thanks again @julien4215 for the hint!

For Android, the server side change has already been done (see https://lichess.org/.well-known/assetlinks.json).

For iOS, the server side change also needs to include the supported paths (see https://docs.flutter.dev/cookbook/navigation/set-up-universal-links#create-and-host-apple-app-site-association-json-file), so this should probably only be done once this PR is merged and we have decided which paths to support.