[WIP] Add zulip:// URI scheme for navigating within app #716

kanishk98 · 2019-04-08T15:41:45Z

What's this PR do?

Registers links starting with zulip:// to open with the Zulip app. Redirects users with installed app to the correct narrow and server (only if the domain is already saved) in the desktop app.

Any background context you want to provide?

Discussion at #470.

Screenshots?

You have tested this PR on:

Windows
Linux/Ubuntu
macOS

andersk · 2019-04-08T20:24:48Z

Is this compatible with the Zulip Android app, which also uses zulip:// URLs? If we’re going to do this, we should make sure to keep them compatible.

kanishk98 · 2019-04-09T01:21:32Z

If we’re going to do this, we should make sure to keep them compatible.

@andersk can you explain what you mean by compatible here?

timabbott · 2019-04-09T17:29:42Z

The mobile app's existing use of zulip:// URLs is only for an authentication flow.

But yes, we should coordinate with the mobile team on what URL syntax we want to use for this because the two apps will end up sharing that syntax.

kanishk98 · 2019-04-10T06:37:12Z

For the benefit of everyone in this thread, I texted Boris on czo yesterday and asked him to have a look at this PR. Waiting for his suggestions. :)

app/main/index.js

akashnimare · 2019-04-12T11:12:12Z

I don't think we need to call the execPath. Also, you need to register the app's protocol in the package.json.
https://github.com/oikonomopo/electron-deep-linking-mac-win
https://stackoverflow.com/questions/43912119/open-app-and-pass-parameters-with-deep-linking-using-electron-macos
https://discuss.atom.io/t/enable-deep-linking/27518/7

kanishk98 · 2019-04-12T17:25:04Z

I don't think we need to call the execPath.

Right, I wasn't sure about this earlier. We should note that this will raise an error in development mode when a developer tries to use this feature because then if Zulip isn't installed the conventional way, it won't find an executable. (happened to me while testing)
I ran npm run dist and worked fine. I'll remove execPath.

Also, you need to register the app's protocol in the package.json.

I don't see the need - the system registers the app's protocol regardless, if we just use the current method. Can you explain why this is required if the feature works both on npm start and npm run dist?

akashnimare · 2019-04-12T18:32:38Z

I don't see the need - the system registers the app's protocol regardless, if we just use the current method.

I think this is required for macOS.

https://stackoverflow.com/questions/45809064/registering-custom-protocol-at-installation-process-in-electron-app

kanishk98 · 2019-04-12T18:44:44Z

I think this is required for macOS.

I checked it on a Mac in debug mode, without creating an executable. It worked well then.
I'll check once more to make sure, and also create an executable for testing.

I'll post back later tonight (I'll have to borrow a Mac, can't do that immediately)

kanishk98 · 2019-04-13T05:35:30Z

I'll post back later tonight

I checked this on a Mac and the protocol is registered, but since the open-url event passes the URL as a string rather than as an array (which is the case on Windows), redirection doesn't happen on macOS. Simple enough problem, I'll push a fix.

kanishk98 · 2019-04-13T13:54:26Z

Alright, I've done a couple of things.

Made sure that the scheme works for Windows, both in dev and production. (the API needs different arguments in different cases for Windows)
Fixed macOS redirection problems.
Added a reload view to make redirection more obvious.

@akashnimare let me know if it works for you now.

abhigyank · 2019-04-15T13:34:01Z

Haven't tested this yet, just out of curiosity from the gif, does this work if the app is not currently running.

kanishk98 · 2019-04-15T13:44:23Z

Haven't tested this yet, just out of curiosity from the gif, does this work if the app is not currently running.

It does, tested that. :)

app/main/index.js

vsvipul · 2019-05-14T09:37:02Z

@kanishk98 i think we only need the URI scheme for the production environment and not for the dev environment. Also, i want to work on this. Ping me if some work is required on this.

kanishk98 · 2019-05-19T03:14:45Z

i think we only need the URI scheme for the production environment and not for the dev environment.

I agree, but it's not really that big of a hassle to include it in the dev environment. Just an extra condition that makes the experience better for Windows developers too.

Also, i want to work on this. Ping me if some work is required on this.

Thanks a ton! I didn't see this earlier, so I ended up testing out electron-builder configs that focus and start the app when the browser receives a zulip:// URL. That seems to work properly right now, and I'm trying to figure how to catch the URL on Linux systems (again, macOS and Windows work well with just the Electron APIs). I'll text you if I get stuck with some Linux stuff.

Assume that Windows executable is stored at C:\Program Files\Zulip\Zulip.exe

Checks if link received by app belongs to a saved domain and switches to the appropriate tab.

akashnimare · 2019-06-10T16:47:41Z

@kanishk98 is this ready to review or still WIP?

kanishk98 · 2019-06-10T16:48:22Z

This is WIP, I encountered a few bugs in the redirect logic that I haven't fixed yet.

vsvipul · 2019-06-12T15:25:22Z

@kanishk98 i went through the PR. What seems to be the issue here? Maybe, i can help.

kanishk98 · 2019-06-12T19:52:33Z

What seems to be the issue here? Maybe, i can help

That'd be great. Basically the app doesn't redirect to the intended URL if it's not running (the URL just opens up the app and then it stays there). I'm working on this right now, so I'll PM you for some help.

Add isURLNarrow() to link-util for identifying which saved domain owns the narrow Fix no-undef error

Also fix redirection issues for macOS. Keep deep linking for both dev and prod builds.

kanishk98 · 2019-07-27T19:19:20Z

To update everyone keeping an eye on this PR, there are some problems with <webview> that cause bugs with the redirection flow in the app. I've not been able to identify them, mostly because they're pretty erratic across operating systems and I think that it's best if I keep this on hold until work with the move to BrowserView is complete. That should address the problems with this PR.

akashnimare · 2019-08-13T20:11:17Z

@kanishk98 are we planning to work on this after BrowserView PR gets merge?

kanishk98 · 2019-08-14T09:06:16Z

are we planning to work on this after BrowserView PR gets merge?

Yeah, I've rechecked the logic for the redirection and the reloading of the <webview> (controlled by the loadURL() method, not us) is pretty erratic. I'd suggest we wait until that happens because we'll have to change things a bit anyway.

zulipbot · 2019-11-13T09:30:30Z

Heads up @kanishk98, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

six8 · 2020-07-24T16:11:04Z

An alternative to this, or addition, would be a way to paste a URL into the Zulip App. I get emails with Zulip App URLs. If I open them in the browser, it loads the web app. The emails would have to use the zulip:// scheme instead of https:// for this PR to work with emails. But that would be confusing if the user isn't using the app.

A way to handle any URL may be to paste the URL into the Zulip App search bar and hit enter and have the Zulip App recognize and go to it.

kanishk98 · 2020-07-28T01:17:08Z

But that would be confusing if the user isn't using the app

Hmm, I'm not sure if I agree with this statement. I'd say that OSes are fairly vocal when redirecting users to desktop apps from URLs - think Windows when it asks if you want to open a Zoom link in the app. Shouldn't that make things obvious to the user?

kanishk98 · 2020-07-28T01:17:48Z

@akashnimare in case we're planning to pick this up this week or so, I can fix the conflicts and work with @manavmehta on this.

frunkad · 2025-01-27T12:00:32Z

Just following, if this has any chance of being picked up again? Though 5 years old, this would be quite an important feature for us - lmk if I can help?

zulipbot added the size: L label Apr 8, 2019

kanishk98 force-pushed the deep-linking branch from 11f33f0 to 564bb7d Compare April 8, 2019 15:52

vsvipul reviewed Apr 11, 2019

View reviewed changes

app/main/index.js Outdated Show resolved Hide resolved

kanishk98 mentioned this pull request Apr 13, 2019

Add URL copy/paster to left sidebar #655

Open

3 tasks

akashnimare reviewed Apr 30, 2019

View reviewed changes

app/main/index.js Outdated Show resolved Hide resolved

kanishk98 added 2 commits May 19, 2019 16:53

deep-linking: Set app as default zulip:// client

92592f3

Assume that Windows executable is stored at C:\Program Files\Zulip\Zulip.exe

deep-linking: Switch to appropriate server

72aec9b

Checks if link received by app belongs to a saved domain and switches to the appropriate tab.

kanishk98 force-pushed the deep-linking branch from 0bef0df to 7d60782 Compare May 19, 2019 11:24

akashnimare changed the title ~~Add zulip:// URI scheme for navigating within app~~ [WIP] Add zulip:// URI scheme for navigating within app Jun 10, 2019

kanishk98 added 3 commits June 13, 2019 03:27

deep-linking: Enable redirection to narrow

c82f8f0

Add isURLNarrow() to link-util for identifying which saved domain owns the narrow Fix no-undef error

webview: show loading indicator on redirection

8c8e299

deep-linking: fix execPath registration issue

576b502

Also fix redirection issues for macOS. Keep deep linking for both dev and prod builds.

deep-linking: Redirect in Linux when app open

dbb8357

kanishk98 force-pushed the deep-linking branch from 7d60782 to dbb8357 Compare June 12, 2019 21:58

kanishk98 added 2 commits June 14, 2019 22:02

deep-linking: Capture zulip:// URL when app closed

a770d3e

deep-linking: Extract protocol code to method

a36c89c

zulipbot added the has conflicts label Nov 13, 2019

Base automatically changed from master to main January 22, 2021 19:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add zulip:// URI scheme for navigating within app #716

[WIP] Add zulip:// URI scheme for navigating within app #716

kanishk98 commented Apr 8, 2019 •

edited

Loading

andersk commented Apr 8, 2019 •

edited

Loading

kanishk98 commented Apr 9, 2019

timabbott commented Apr 9, 2019

kanishk98 commented Apr 10, 2019

akashnimare commented Apr 12, 2019

kanishk98 commented Apr 12, 2019 •

edited

Loading

akashnimare commented Apr 12, 2019

kanishk98 commented Apr 12, 2019

kanishk98 commented Apr 13, 2019

kanishk98 commented Apr 13, 2019 •

edited

Loading

abhigyank commented Apr 15, 2019

kanishk98 commented Apr 15, 2019

vsvipul commented May 14, 2019

kanishk98 commented May 19, 2019 •

edited

Loading

akashnimare commented Jun 10, 2019

kanishk98 commented Jun 10, 2019

vsvipul commented Jun 12, 2019

kanishk98 commented Jun 12, 2019

kanishk98 commented Jul 27, 2019

akashnimare commented Aug 13, 2019

kanishk98 commented Aug 14, 2019

zulipbot commented Nov 13, 2019

six8 commented Jul 24, 2020

kanishk98 commented Jul 28, 2020

kanishk98 commented Jul 28, 2020

frunkad commented Jan 27, 2025

[WIP] Add zulip:// URI scheme for navigating within app #716

Are you sure you want to change the base?

[WIP] Add zulip:// URI scheme for navigating within app #716

Conversation

kanishk98 commented Apr 8, 2019 • edited Loading

andersk commented Apr 8, 2019 • edited Loading

kanishk98 commented Apr 9, 2019

timabbott commented Apr 9, 2019

kanishk98 commented Apr 10, 2019

akashnimare commented Apr 12, 2019

kanishk98 commented Apr 12, 2019 • edited Loading

akashnimare commented Apr 12, 2019

kanishk98 commented Apr 12, 2019

kanishk98 commented Apr 13, 2019

kanishk98 commented Apr 13, 2019 • edited Loading

abhigyank commented Apr 15, 2019

kanishk98 commented Apr 15, 2019

vsvipul commented May 14, 2019

kanishk98 commented May 19, 2019 • edited Loading

akashnimare commented Jun 10, 2019

kanishk98 commented Jun 10, 2019

vsvipul commented Jun 12, 2019

kanishk98 commented Jun 12, 2019

kanishk98 commented Jul 27, 2019

akashnimare commented Aug 13, 2019

kanishk98 commented Aug 14, 2019

zulipbot commented Nov 13, 2019

six8 commented Jul 24, 2020

kanishk98 commented Jul 28, 2020

kanishk98 commented Jul 28, 2020

frunkad commented Jan 27, 2025

kanishk98 commented Apr 8, 2019 •

edited

Loading

andersk commented Apr 8, 2019 •

edited

Loading

kanishk98 commented Apr 12, 2019 •

edited

Loading

kanishk98 commented Apr 13, 2019 •

edited

Loading

kanishk98 commented May 19, 2019 •

edited

Loading