Skip to content

Linking API: Support for New Tab#1432

Closed
shahzore-qureshi wants to merge 2 commits intonecolas:masterfrom
shahzore-qureshi:patch-1
Closed

Linking API: Support for New Tab#1432
shahzore-qureshi wants to merge 2 commits intonecolas:masterfrom
shahzore-qureshi:patch-1

Conversation

@shahzore-qureshi
Copy link
Copy Markdown

I added support for opening a link in a new tab.

The second parameter "target" can accept the following values:
"_blank" : URL is loaded into a new window or tab (default)
"_self" : URL replaces the current page

I added support for opening a link in a new tab.

The second parameter "target" can accept the following values:
"_blank" : URL is loaded into a new window or tab (default)
"_self" : URL replaces the current page
@swyxio
Copy link
Copy Markdown

swyxio commented Sep 15, 2019

instead of target: string, you can use string literals: target: "_blank" | "_self

@shahzore-qureshi
Copy link
Copy Markdown
Author

Good call @sw-yx! Thank you for the feedback.

@shahzore-qureshi shahzore-qureshi changed the title Linking: Support for New Tab Linking API: Support for New Tab Sep 16, 2019
@EyMaddis
Copy link
Copy Markdown

EyMaddis commented Nov 9, 2019

I would propose a settings object as the second parameter, like:

Linking.openURL('some_url', { 
  target: '_blank', 
  rel: 'noopener' 
})

I think it is safe to assume that more properties might follow.

@gianpaj
Copy link
Copy Markdown

gianpaj commented Jan 12, 2020

Any update on this?

@brunolemos
Copy link
Copy Markdown
Contributor

I don't think this belongs at this repository. React Native only receives the url param, so react-native-web should be the same. You can easily implement this on user land. For example, in my case, I called it Browser.openURL instead of Linking and made it support the params I want.

@shahzore-qureshi
Copy link
Copy Markdown
Author

shahzore-qureshi commented Jan 16, 2020

Sorry for the delay!

I agree @EyMaddis . I can add a settings object to that call.

@brunolemos - since react-native-web is a separate repository and is targeted for the web (and not mobile apps), I am okay with differentiating react-native from react-native-web.

Also, "Linking" on the web is quite different from "Linking" on mobile. Therefore, I believe the "Linking" module should have better support for the web. In addition, why even have the "Linking" module in the react-native-web repository if we end up using a made-up "Browser" module that you mentioned? Perhaps I am missing the point of the "Linking" module.

@EyMaddis
Copy link
Copy Markdown

By the way, I totally agree with @brunolemos

I don't think this belongs at this repository. React Native only receives the url param, so react-native-web should be the same. You can easily implement this on user land.

@shahzore-qureshi
Copy link
Copy Markdown
Author

@EyMaddis - I do not agree with @brunolemos due to the reasoning in my last comment.

@kopax
Copy link
Copy Markdown

kopax commented Jan 22, 2020

Can this be merged? I look forward for more convenient tools. How do you suggest to do userland?

@Alercard
Copy link
Copy Markdown

Alercard commented May 7, 2020

and if we use this code:
if (Platform.OS==='web') {
window.open(link, '_blank');
}else{
Linking.canOpenURL(link).then(supported => {
if (supported) {
Linking.openURL(link);
} else {
console.log("Don't know how to open URI: " + link);
}
});
}

@EvanBacon
Copy link
Copy Markdown
Collaborator

Perhaps this should be in a package like expo-web-browser which supports opening external web pages on native (similar to creating a new tab).

@soroushm
Copy link
Copy Markdown

soroushm commented Aug 5, 2020

its gonna be merge?

@necolas necolas added this to the 0.15 milestone Dec 8, 2020
@necolas necolas closed this in b7b3830 Feb 12, 2021
rnike pushed a commit to VeryBuy/react-native-web that referenced this pull request Sep 13, 2022
@shahzore-qureshi
Copy link
Copy Markdown
Author

shahzore-qureshi commented Mar 2, 2023

@necolas and @avanwinkle - it looks like my original PR has finally been implemented as of this commit: ffd300e. May I ask what changed your minds on this topic? Why did you decide to implement the "target" parameter about 3 years after my request? And why was there a commit that hardcoded the target instead of allowing a target parameter? I am referring to this commit: b7b3830. I am genuinely curious and also frustrated with the direction of this project. It would be great to have some transparency on how these decisions are made since they impact so many developers around the world. I get that Meta is the owner of this initiative, but this is also an open-source project, and I request that Meta treat other developers on GitHub as potential contributors and not just issue reporters. Thanks in advance!

@shahzore-qureshi
Copy link
Copy Markdown
Author

In addition, are there plans to officially move the React Native Web repo from your personal GitHub account to the official Meta/Facebook organization? I am concerned that this project is not receiving much attention because it is not officially under the Meta/Facebook organization. I see so much potential with this project, and I really want it to succeed.

@kidroca
Copy link
Copy Markdown

kidroca commented Mar 2, 2023

I believe the reason to not accept your proposal earlier is because it didn't match the interface of Linking of react-native and it was relatively easy to code this in user land

But later it was decided it's convenient to make linking open everything in new window/tab
And this didn't change the interface

But then users started to complain that they don't want to open everything in new tab

So it seems in order to keep the feature of opening in new tab by default. But satisfy the rest that didn't want that a target parameter was added


My observations are that developer conveniences aren't a priory for this project
(Or there's simply too little time / maintainers)

@necolas
Copy link
Copy Markdown
Owner

necolas commented Mar 7, 2023

You're complaining that 1) you wanted this feature and it was added, and 2) that you want contributor PRs to be merged and a contributor PR was merged? What you're implying is not supported by facts.

This project has dozens of contributor PRs merged every release. If people submit good PRs with explanations, tests, etc., (as requested in the contributing guidelines) then it makes it a lot easier to review and merge. Unfortunately a lot of PRs lack context, change too many things for prompt review, and have little to no tests. And the person who improves (e.g., add the unit tests you didn't) and then maintains those contributor patches "forever" is me, not you.

The only reason this project is still going is because I keep it going. There's little to no support from Meta engineering. This project has to dance between supporting RN APIs that frequently had no thought for web, and what web devs want to do via React DOM with little thought for compat with React Native. Not to mention that I've put a lot of work into landing changes to React Native so more web code works out-of-the-box on React Native in the future.

Keep that in mind next time you feel like venting. Thanks in advance!

Repository owner locked as resolved and limited conversation to collaborators Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.