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

Handle Zulip-internal links by navigation #73

Closed
gnprice opened this issue Apr 19, 2023 · 2 comments · Fixed by #318
Closed

Handle Zulip-internal links by navigation #73

gnprice opened this issue Apr 19, 2023 · 2 comments · Fixed by #318
Assignees
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents a-msglist The message-list screen, except what's label:a-content
Milestone

Comments

@gnprice
Copy link
Member

gnprice commented Apr 19, 2023

This is a followup to #71 that will only make sense after we have #72 as well.

Once we recognize the user tapping on a link in a message (#71), and we have UI for a variety of narrows (#72), we'll want to start recognizing when a link refers to another Zulip narrow. Then we can follow the link by navigating to that narrow within the app, rather than opening it in a browser.

This is a key part of the UX when reading a conversation that refers to other conversations and messages, because opening them in a browser will typically mean having to log into Zulip anew.

@gnprice gnprice added this to the Alpha milestone May 27, 2023
@gnprice gnprice added a-msglist The message-list screen, except what's label:a-content a-content Parsing and rendering Zulip HTML content, notably message contents and removed m-alpha labels May 27, 2023
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 12, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 13, 2023
We can use this to get raw Markdown content for quote-and-reply
(zulip#116) and for the "Share" option on a message. For those, we only
care about the raw Markdown content and so could just as well have
used the `raw_content` field on the get-single-message response, for
servers pre-120. But...

We can also use this for zulip#73, "Handle Zulip-internal links by
navigation", to follow /near/<id> links through topic/stream moves
(see implementation in zulip-mobile). For that, we'll need more than
just the message's raw Markdown.
@gnprice
Copy link
Member Author

gnprice commented Jul 12, 2023

The prerequisites #71 and #72 are complete. I believe what's needed for this issue is now:

  • Move narrowLink and its private helpers out of lib/model/compose.dart and into their own file, like lib/model/internal_link.dart.
  • In that file, add a sort of inverse to narrowLink: a function that takes a link and converts it to a Narrow value, returning null if the link isn't to a narrow on the current realm.
    • This will definitely want unit tests.
  • Then when the user taps a link to open it (msglist: Make links touchable, opening in browser #71), before we go and try to launch the URL in a browser, try to parse it to a Narrow. If that succeeds, navigate to that narrow, in the same way as we do when the user touches a recipient header.

Two aspects I'd like to leave out of scope for this issue are:

@gnprice
Copy link
Member Author

gnprice commented Aug 3, 2023

The structure of these links is currently a gap in Zulip's API documentation. See chat discussions:
https://chat.zulip.org/#narrow/stream/378-api-design/topic/id.3A123.20narrow.20in.20GET.20.2Fmessages/near/1609232
https://chat.zulip.org/#narrow/stream/412-api-documentation/topic/Narrow.20URL.20formats/near/1619527

So it remains an area where one has to resort to reverse-engineering.

  • Most of the information one needs is contained just in looking at narrowLink and trying to construct an inverse for it.
  • The zulip-mobile implementation of parsing these links is getNarrowFromNarrowLink in src/utils/internalLinks.js . I wouldn't want to copy the structure of that implementation — in particular it's unnecessarily brittle in assuming that the different operators come in a specific order. But it does contain at least one useful piece of information which wouldn't be apparent from trying to invert narrowLink: old servers would generate links with subject/ where today we'd say topic/, and consequently we'll want to understand such links because they may still appear in old messages. (Thanks to Neil Pilgrim for spotting that.)
  • The web client should also be a useful resource to consult.

sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Sep 15, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Sep 21, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Sep 25, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Sep 25, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Sep 25, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Sep 25, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 4, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 4, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 5, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 9, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 11, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 12, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 13, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 13, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 13, 2023
Integrates internal_links into link nodes
so that urls that resolve to internal Narrows
navigate to that instead of launching in an
external browser.

Fixes: zulip#73
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 16, 2023
Integrates [parseInternalLink] into content link nodes
so that recognized narrows are navigated to within the app,
rather than opened in a browser.

Fixes: zulip#73
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents a-msglist The message-list screen, except what's label:a-content
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants