Skip to content

Open urls in browser not WebView#4336

Closed
shaurya2612 wants to merge 5 commits intozulip:masterfrom
shaurya2612:4323
Closed

Open urls in browser not WebView#4336
shaurya2612 wants to merge 5 commits intozulip:masterfrom
shaurya2612:4323

Conversation

@shaurya2612
Copy link
Copy Markdown
Contributor

#4323 replaced NativeModules with Linking to open the url in a browser instance instead of a WebView

replaced NativeModules with Linking to open the url in a browser instance instead of a WebView
@shaurya2612
Copy link
Copy Markdown
Contributor Author

shaurya2612 commented Dec 12, 2020

The linking API helps in opening links straight away in the browser and not in webview, thus eliminating extra steps which share info between Zulip and the default browser of the device. (mentioned in #4323)

iOS Example -
Previous WebView Screen -
Screen Shot 2020-12-12 at 12 42 40 PM

After using the Linking API -
Screen Shot 2020-12-12 at 12 43 42 PM

@gnprice
Copy link
Copy Markdown
Member

gnprice commented Dec 15, 2020

Thanks @shaurya2612 for the PR and the screenshots!

Here's a few comments on this change:

  • This is something that'll be different between Android and iOS -- in fact the existing code has a conditional to do different things for each one. So we'll need to evaluate the change separately for each platform. To keep things simple, let's focus on just one for now, so let's start with a version that doesn't change what we do on Android.
  • This openLink function is used in a number of places in the app. You can find them through the IDE, or with a command-line search like rg -C2 'openLink\('. We should make sure the change makes sense for all of them and won't break how they work.
    • In particular there are two calls to it in our code for signing into a Zulip account, in AuthScreen.js and webAuth.js. I suspect that for those two, using an external browser will be a worse experience, and we should stick with the embedded Safari view for those.
    • There's also a couple of calls in our code for the lightbox, where the user is trying to download or share an image, and there's a call in messageLinkPress that corresponds to when the user taps on an image that's part of a message. Here I'm not sure if the external browser will be a better or worse experience.

We can have two versions of this function, named like openLink and openLinkEmbedded, where the latter uses a Safari view. (And, at least for now, on Android both functions will still use an embedded browser view.) Then we can call the two versions in different places.

Then the main work that'll be needed in order to finish this from here is to go through each of the different flows that use openLink to open a link, and test how the experience is with this change so we can decide which ones it makes sense to use the external browser for and which ones we should continue to use the Safari view for.

replaced NativeModules with Linking to open the url in a browser instance instead of a WebView
Made previous webview behaviour persist for android and created two seperate functions (openLink and openLinkEmbedded) for different use cases in iOS
@shaurya2612
Copy link
Copy Markdown
Contributor Author

This PR has been continued at #4343 as the current branch faced some rebasing issues.

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.

2 participants