Add support for opening lemmy links #832

ggichure · 2023-10-16T08:04:56Z

Pull Request Description

Add support for opening Lemmy links in app using uni_links

Users e.g https://lemmy.world/u/lwadmin
Posts e.g https://lemmy.world/post/6740849
Comments e.g https://lemmy.world/comments/1234567
Depends on Support navigating to posts from within the app #818
I'll need help with the scripting part to auto-gen instances. Automatically populate Lemmy domains in Android manifest #834

Android support for now.

Issue Being Fixed

Issue Number: #60

Screenshots / Recordings

Checklist

Did you update CHANGELOG.md?
Did you use localized strings where applicable?
Did you add semanticLabels where applicable for accessibility?

micahmo · 2023-10-16T15:00:58Z

This is awesome, thanks for working on this!!

I know it's still a draft, so I didn't officially review, but I wanted to jot down some thoughts before I forgot. Note that I'm not asking for you to do any of these things. Just notes for reference. 😊

As you mentioned, it only supports Android for now (maybe @hjiangsu can implement for iOS as described here).
As you mentioned, the list of supported domains is hard-coded. We can help to auto-generate the list in the instance workflow.
Any reason for picking uni_links over app_links? (I haven't researched either one so I'm asking purely out of curiosity.)
We should have graceful handling of any non-post link (I'm assuming we're only supporting posts to start).
Does the user need to manually enable link handling for each supported domain in the app's Android settings? (I believe that's required unless an app publisher can verify that it owns a domain.) If so, we should document that process.

Again, super awesome, and please don't take any comments as criticism or asking for extra work. 😊

ggichure · 2023-10-16T15:52:22Z

Any reason for picking uni_links over app_links?

I have in the past used uni_links

We should have graceful handling of any non-post link (I'm assuming we're only supporting posts to start).

Correct, only posts for now. Although, I'll look into the comments and user profiles too.

Does the user need to manually enable link handling for each supported domain in the app's Android settings? (I believe that's required unless an app publisher can verify that it owns a domain.) If so, we should document that process.

Given that we are using deep links we don't have to verify we own the domains. Deep links are general links that point to specific content within an app.

App links on the other hand are designed to verify and ensure that links are opened within the designated app rather than through a web browser, if the app is installed.

Again, super awesome, and please don't take any comments as criticism or asking for extra work.

I thoroughly enjoy working on Thunder, and I welcome constructive criticism.

micahmo · 2023-10-16T16:01:56Z

Does the user need to manually enable link handling for each supported domain in the app's Android settings? (I believe that's required unless an app publisher can verify that it owns a domain.) If so, we should document that process.

Given that we are using deep links we don't have to verify we own the domains. Deep links are general links that point to specific content within an app.

App links on the other hand are designed to verify and ensure that links are opened within the designated app rather than through a web browser, if the app is installed.

I understand that we don't need to (and can't) verify all the domains. However, I believe we still need to instruct users to enable link handling for all of those domains in the Android settings. For example, if I grab and run your branch, Thunder does not automatically handle those domains. I have to enable them, like this.

qemu-system-x86_64_ow54t3xYYh.mp4

This is what I mean when I say we should document the process to enable link handling (unless I am misunderstanding something!).

I thoroughly enjoy working on Thunder, and I welcome constructive criticism.

Me too. 😊 Thanks for all the help!

ggichure · 2023-10-16T16:48:50Z

Does the user need to manually enable link handling for each supported domain in the app's Android settings? (I believe that's required unless an app publisher can verify that it owns a domain.) If so, we should document that process.

Given that we are using deep links we don't have to verify we own the domains. Deep links are general links that point to specific content within an app.
App links on the other hand are designed to verify and ensure that links are opened within the designated app rather than through a web browser, if the app is installed.

I understand that we don't need to (and can't) verify all the domains. However, I believe we still need to instruct users to enable link handling for all of those domains in the Android settings. For example, if I grab and run your branch, Thunder does not automatically handle those domains. I have to enable them, like this.
qemu-system-x86_64_ow54t3xYYh.mp4

This is what I mean when I say we should document the process to enable link handling (unless I am misunderstanding something!).

I thoroughly enjoy working on Thunder, and I welcome constructive criticism.

Me too. 😊 Thanks for all the help!

Yeah, we need to add a guide.

micahmo · 2023-10-17T04:29:27Z

Hey again! I opened PR #834 to help with auto-generating the supported domain list in the Android manifest file via the GitHub workflows. It assumes that the file already contains...

...
<intent-filter>
  <action android:name="android.intent.action.VIEW" />
  <category android:name="android.intent.category.DEFAULT" />
  <category android:name="android.intent.category.BROWSABLE" />
  <data android:scheme="https"/>
   <!-- TODO(lemmy_server_domain_finder): Setup a script to update available lemmy domains  -->
  <!-- Obtained from github.com/dessalines/jerboa/blob/main/app/src/main/AndroidManifest.xml-->
  <!--#AUTO_GEN_INSTANCE_LIST_DO_NOT_TOUCH#-->
  ...
  <data android:host="WHATEVER" />
  ...
  <!--#INSTANCE_LIST_END#-->
</intent-filter>
...

We can either...

Copy my changes and incorporate them into your PR
Do my PR separately from your changes

I have no preference. :)

…links.

ggichure · 2023-10-17T07:49:30Z

Hey again! I opened PR #834 to help with auto-generating the supported domain list in the Android manifest file via the GitHub workflows. It assumes that the file already contains...

...
<intent-filter>
  <action android:name="android.intent.action.VIEW" />
  <category android:name="android.intent.category.DEFAULT" />
  <category android:name="android.intent.category.BROWSABLE" />
  <data android:scheme="https"/>
   <!-- TODO(lemmy_server_domain_finder): Setup a script to update available lemmy domains  -->
  <!-- Obtained from github.com/dessalines/jerboa/blob/main/app/src/main/AndroidManifest.xml-->
  <!--#AUTO_GEN_INSTANCE_LIST_DO_NOT_TOUCH#-->
  ...
  <data android:host="WHATEVER" />
  ...
  <!--#INSTANCE_LIST_END#-->
</intent-filter>
...

We can either...

* Copy my changes and incorporate them into your PR

* Do my PR separately from your changes

I have no preference. :)

Your PR can go in separately.
Thanks for this.

ggichure · 2023-10-17T07:58:24Z

@micahmo @hjiangsu

I have a commentId , navigateToPost requires at least a postId or PostViewMedia , any insights on how to navigate user to PostPage just using commentId?

micahmo · 2023-10-17T11:38:54Z

I have a commentId , navigateToPost requires at least a postId or PostViewMedia , any insights on how to navigate user to PostPage just using commentId?

I had intended to do a followup to #818 by refactoring a navigateToComment method and using it for in-app navigation as well (you can see my TODO here). Let me see if I can knock that out for you!

EDIT: #835 adds a navigateToComment method. 😊

P.S. I was thinking you could use the instance page from #824 if they navigate to the root domain and maybe also if they navigate to an unsupported link.

hjiangsu · 2023-10-17T22:43:40Z

This is pretty awesome, thanks again for initiating work on this! I don't have too much experience with integrating deep links but I'll see what I can do on the iOS side of things to make it work as well.

This is what I mean when I say we should document the process to enable link handling (unless I am misunderstanding something!).

I wonder if we can make it easier by having a button which opens up the app settings page for the user to be able to select the applicable domains? I'm not sure if this is possible but it would be a nice QoL feature!

ggichure · 2023-10-18T02:47:53Z

I wonder if we can make it easier by having a button which opens up the app settings page for the user to be able to select the applicable domains? I'm not sure if this is possible but it would be a nice QoL feature!

Using permission_handler you can open app settings.

micahmo · 2023-10-18T02:56:44Z

I don't have too much experience with integrating deep links but I'll see what I can do on the iOS side of things

Hopefully it should just involve creating/editing an entitlements file, and the code should be the same for both platforms.

I wonder if we can make it easier by having a button which opens up the app settings page for the user to be able to select the applicable domains?

That's a great idea, and I'm pretty sure it's possible on Android!

ggichure · 2023-10-18T06:06:59Z

Open settings done.

ggichure · 2023-10-18T07:58:37Z

@micahmo
getLemmyCommentId ResolveObject doesn't "always" work . instance.dart#L114

Default instance being used lemmy.ml
On lemmy.world it works.
https://lemmy.world/comment/3078559

 final ResolveObjectResponse resolveObjectResponse = await lemmy.run(ResolveObject(q: text));

LemmyApiException: couldnt_find_object

micahmo · 2023-10-18T14:13:03Z

Open settings done.

That's awesome! Is there any way to get it to the specific settings page for links? Maybe not with permission_manager but maybe with app_settings? Also do you want to add an icon to that setting so that it aligns with the other ones?

getLemmyCommentId ResolveObject doesn't "always" work .

It won't be able to resolve an object that is not federated to the current instance. I'm not sure what we can do about that. Maybe just show a message? We could still open it in the browser in the app I suppose.

ggichure · 2023-10-18T14:40:43Z

app_settings seems like it will do the job.

Edit:
app_settings doesn't launch specific settings page for links.

Maybe just show a message? We could still open it in the browser in the app I suppose.

Sure.

Also do you want to add an icon to that setting so that it aligns with the other ones?

Sure

hjiangsu · 2023-10-18T15:36:39Z

Hopefully it should just involve creating/editing an entitlements file, and the code should be the same for both platforms.

So I've played around a little bit with this yesterday, and unfortunately, there doesn't seem to be a good way to handle this on iOS (that I know of so far).

Using Universal Links means that we would need to verify the domains we add to the entitlements/info.plist files. However, since we don't directly own those domains, thats not a possibility.

We can use Custom URL schemes, but it doesn't work entirely the same as it does on Android. What happens here is that we need to add any unique custom url schemes that we want to associate with our app which also does not clash with another installed app (e.g., thunder://). I tried to test out using https://lemmy.world, but it seems to open in Safari because the http/https schemes are handled by Safari by default and there doesn't seem to be a way to override that from what I can tell.

What would need to happen then in this case, is that we would need to have users replace the https:// portion with thunder:// for it to then redirect the user to the app

micahmo · 2023-10-18T16:45:11Z

Using Universal Links means that we would need to verify the domains we add to the entitlements/info.plist files.

Ah man, that's too bad. I was hoping it would work like Android where the domains are simply disabled by default. But it makes sense from a security perspective why Apple would be a little more restrictive about this. Oh well!

micahmo

Hey @ggichure, since you marked this ready for review, I went ahead and reviewed. Hope that's ok. 😊 Looks great overall! I left a couple comments on the code. I also took this for a test run.

The UI for the new setting still looks a bit funny, with the leading icon misaligned and the trailing very large. It also doesn't have the same stadium border and inkwell as the rest of the settings. Not sure if it's possible to reuse one of the existing setting widgets with a custom action. Also it still just opens to the main settings page (if you did not intend to change that in this PR, that's totally fine!!).

The main issue is that things don't seem to be working very well for me! I'm sure I'm doing something wrong, but here's what I'm seeing. This is with a post on the same instance that I'm logged in with, so shouldn't be a federation issue. (The same thing happens whether Thunder is already open or not.)

qemu-system-x86_64_OHz2PhdXB8.mp4

P.S. Thanks as always for the additional localization.

lib/utils/navigate_post.dart

lib/utils/navigate_to_comment.dart

ggichure · 2023-10-18T18:49:15Z

Also it still just opens to the main settings page (if you did not intend to change that in this PR, that's totally fine!!)

So far I haven't found a way to send the user to the page we need .

ggichure · 2023-10-19T08:29:48Z

The UI for the new setting still looks a bit funny, with the leading icon misaligned and the trailing very large. It also doesn't have the same stadium border and inkwell as the rest of the settings. Not sure if it's possible to reuse one of the existing setting widgets with a custom action.

Reused the ToggleOption to create a SettingsListTile.

ggichure · 2023-10-19T08:31:40Z

The main issue is that things don't seem to be working very well for me! I'm sure I'm doing something wrong, but here's what I'm seeing. This is with a post on the same instance that I'm logged in with, so shouldn't be a federation issue. (The same thing happens whether Thunder is already open or not.)

I'm unable to reproduce this.
Also I'm considering increasing the snackbar duration on link exception. .

hjiangsu · 2023-10-19T17:55:19Z

Just did a quick code review and everything seems to be fine with me, nothing too much to point out aside from what was already discussed previously (thanks @micahmo for doing a code review as well!)

I unfortunately can't test this myself since I don't have a physical Android device, so I'll leave it up to @micahmo to approve of this if thats okay

micahmo

If @ggichure can't reproduce the issue I was seeing, then I approve! Might as well get it into the nightly and get some feedback. 😊

hjiangsu · 2023-10-19T18:08:17Z

Sounds good! Just to confirm, @ggichure is this PR ready to go?

ggichure · 2023-10-19T18:11:03Z

Sounds good! Just to confirm, @ggichure is this PR ready to go?

Yes it Is.

init add support for opening lemmy links

23215e9

micahmo mentioned this pull request Oct 17, 2023

Automatically populate Lemmy domains in Android manifest #834

Merged

ggichure and others added 2 commits October 17, 2023 08:03

Merge branch 'thunder-app:develop' into feature/open_lemmy_links_in_app

17beece

use cubits to handle deep links, handle navigate to post and to user …

780db70

…links.

micahmo mentioned this pull request Oct 17, 2023

Support navigating to comments from within the app #835

Merged

3 tasks

ggichure and others added 2 commits October 18, 2023 07:59

Merge branch 'develop' into feature/open_lemmy_links_in_app

d9424b5

navigate to comments

f380983

ggichure changed the title ~~Initial add support for opening lemmy links~~ Add support for opening lemmy links Oct 18, 2023

ggichure added 2 commits October 18, 2023 09:12

open app settings

1c0364e

localize general settings page

652ff4b

mv accountbloc to main.dart

d2034d2

UI fix open links

838ce5c

snackbars on unresolved links and open link, updated changelog

7d5dea4

ggichure marked this pull request as ready for review October 18, 2023 15:44

hjiangsu mentioned this pull request Oct 18, 2023

Deep link protocol #146

Open

micahmo reviewed Oct 18, 2023

View reviewed changes

lib/utils/navigate_post.dart Show resolved Hide resolved

lib/utils/navigate_post.dart Show resolved Hide resolved

lib/utils/navigate_to_comment.dart Outdated Show resolved Hide resolved

micahmo mentioned this pull request Oct 19, 2023

Disable overscroll effect on Android when reduce animations setting is enabled #839

Merged

3 tasks

rm unused, fix settings tile UI

159e2e0

ggichure and others added 2 commits October 19, 2023 19:50

Merge branch 'develop' into feature/open_lemmy_links_in_app

31dcef0

uri exceptions snackbars lasts 10 seconds

4300409

micahmo approved these changes Oct 19, 2023

View reviewed changes

hjiangsu merged commit e57228a into thunder-app:develop Oct 19, 2023

ggichure mentioned this pull request Oct 20, 2023

Ability to see the destination of any link before visiting #309

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for opening lemmy links #832

Add support for opening lemmy links #832

ggichure commented Oct 16, 2023 •

edited

Loading

micahmo commented Oct 16, 2023

ggichure commented Oct 16, 2023

micahmo commented Oct 16, 2023

ggichure commented Oct 16, 2023

micahmo commented Oct 17, 2023

ggichure commented Oct 17, 2023

ggichure commented Oct 17, 2023

micahmo commented Oct 17, 2023 •

edited

Loading

hjiangsu commented Oct 17, 2023

ggichure commented Oct 18, 2023

micahmo commented Oct 18, 2023 •

edited

Loading

ggichure commented Oct 18, 2023

ggichure commented Oct 18, 2023

micahmo commented Oct 18, 2023

ggichure commented Oct 18, 2023 •

edited

Loading

hjiangsu commented Oct 18, 2023

micahmo commented Oct 18, 2023

micahmo left a comment •

edited

Loading

ggichure commented Oct 18, 2023

ggichure commented Oct 19, 2023

ggichure commented Oct 19, 2023

hjiangsu commented Oct 19, 2023

micahmo left a comment

hjiangsu commented Oct 19, 2023

ggichure commented Oct 19, 2023

Add support for opening lemmy links #832

Add support for opening lemmy links #832

Conversation

ggichure commented Oct 16, 2023 • edited Loading

Pull Request Description

Issue Being Fixed

Screenshots / Recordings

Checklist

micahmo commented Oct 16, 2023

ggichure commented Oct 16, 2023

micahmo commented Oct 16, 2023

ggichure commented Oct 16, 2023

micahmo commented Oct 17, 2023

ggichure commented Oct 17, 2023

ggichure commented Oct 17, 2023

micahmo commented Oct 17, 2023 • edited Loading

hjiangsu commented Oct 17, 2023

ggichure commented Oct 18, 2023

micahmo commented Oct 18, 2023 • edited Loading

ggichure commented Oct 18, 2023

ggichure commented Oct 18, 2023

micahmo commented Oct 18, 2023

ggichure commented Oct 18, 2023 • edited Loading

hjiangsu commented Oct 18, 2023

micahmo commented Oct 18, 2023

micahmo left a comment • edited Loading

Choose a reason for hiding this comment

ggichure commented Oct 18, 2023

ggichure commented Oct 19, 2023

ggichure commented Oct 19, 2023

hjiangsu commented Oct 19, 2023

micahmo left a comment

Choose a reason for hiding this comment

hjiangsu commented Oct 19, 2023

ggichure commented Oct 19, 2023

ggichure commented Oct 16, 2023 •

edited

Loading

micahmo commented Oct 17, 2023 •

edited

Loading

micahmo commented Oct 18, 2023 •

edited

Loading

ggichure commented Oct 18, 2023 •

edited

Loading

micahmo left a comment •

edited

Loading