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

Implement !community@instance and @user@instance links #683

Closed

Conversation

beatgammit
Copy link
Contributor

@beatgammit beatgammit commented Jun 15, 2023

They merely pop up a toast saying it's unimplemented, but the links are being created and handled.

Fixes #556.

Partial fix for #526 since short forms of lemmy links can be handled separately from other links.

Related to #454.

@beatgammit beatgammit marked this pull request as draft June 15, 2023 22:03
@beatgammit beatgammit force-pushed the feat/implement-lemmy-links branch 2 times, most recently from 9640160 to 38425c2 Compare June 15, 2023 22:09
@twizmwazin
Copy link
Contributor

This looks really great! Look forward to when its ready.

@beatgammit beatgammit force-pushed the feat/implement-lemmy-links branch from 38425c2 to f66bafa Compare June 15, 2023 22:15
@beatgammit
Copy link
Contributor Author

beatgammit commented Jun 15, 2023

I'm happy to merge as-is, or I can take a couple days and figure out how to open community and user pages.

If #583 gets merged, it'll at least prompt how to open it, which is better than what we have now.

Preference?

@beatgammit beatgammit force-pushed the feat/implement-lemmy-links branch from f66bafa to d26bb05 Compare June 15, 2023 22:39
They merely pop up a toast saying it's unimplemented, but the links are
being created and handled.
@beatgammit beatgammit force-pushed the feat/implement-lemmy-links branch from d26bb05 to 892f0f3 Compare June 16, 2023 00:00
@@ -423,6 +423,14 @@ fun parseUrl(url: String): String? {
fun openLink(url: String, ctx: Context, useCustomTab: Boolean, usePrivateTab: Boolean) {
val url = parseUrl(url) ?: return

if (url.startsWith("@")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Following #583, I don't think this section triggers. parseUrl will return a standard https link. This should probably be rewritten to check if a link appears to be a user or community link, and then handle it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked through your changes, and you may want to consider setting navController outside of the MarkdownHelper.init() so it can easily be used in previews. What I did was make it nullable and made a setter for it, and then check it before passing it on to openLinks.

Just an idea, I'm certainly no expert here. 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think you're right, would welcome a PR for it.

@twizmwazin
Copy link
Contributor

I merged this as part of #692, thank you!

@twizmwazin twizmwazin closed this Jun 16, 2023
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.

Crash on opening specific link
2 participants