Universal Linking component by chirag04 · Pull Request #4546 · react/react-native

chirag04 · 2015-12-03T19:00:40Z

A lot of code is redundant in IntentAndroid and LinkingIOS. This bridges that gap.
Not sure what we should do with IntentAndroid.js and LinkingIOS.js if we merge this.

cc @mkonicek @ide @brentvatne

facebook-github-bot · 2015-12-03T19:01:26Z

By analyzing the blame information on this pull request, we identified @philikon, @nicklockwood and @vjeux to be potential reviewers.

vjeux · 2015-12-03T19:07:07Z

cc @dmmiller who's running the effort of unifying iOS and Android apis :)

philikon · 2015-12-03T19:08:26Z

Could the diff be expressed as a rename of LinkingIOS.js to Linking.js please?

satya164 · 2015-12-04T02:38:20Z

I think we should show a warning when someone tries to use the events on Android.

Also, deep linking on Android is not supported on master, I've a pull request open here #4320 . However, I need to test it with my latest changes before I get it merged.

satya164 · 2015-12-04T02:43:19Z

BTW, currently we have 3 different files, LinkingIOS, IntentAndroid and Linking. We could merge them to a single Linking.js file and just export it as LinkingIOS, IntentAndroid and Linking for backward compatibility.

nicklockwood · 2015-12-04T15:46:28Z

Is "Linking" a good name for this? I can't say I'm a fan of it. If we're renaming it anyway, might as well pick something we like.

satya164 · 2015-12-04T15:48:12Z

@nicklockwood Any ideas for a Good name?

nicklockwood · 2015-12-04T15:56:10Z

Something like Routing? or URLRouting?

I don't know. Names are hard ¯_(ツ)_/¯

satya164 · 2015-12-04T15:57:52Z

@nicklockwood Routing sounds like it has something to do with React (Native) Router :|

Yeah, names are pretty damn hard :(

chirag04 · 2015-12-04T16:37:22Z

Agree with you guys. How about url or history?

@satya164: having one file and exporting it as linkingios, intentAndroid etc sounds like a good plan

satya164 · 2015-12-04T16:39:34Z

@chirag04 Nah, URL means, well URL (URL.openURL(url: string) is kinda weird), and History is more like what the Navigators does.

chirag04 · 2015-12-04T16:41:16Z

yeah. let's wait for more suggestions. I will update the PR accordingly. My plan was to start a discussion around this with this PR :)

mkonicek · 2015-12-09T22:02:36Z

Thanks for doing this! We should remove LinkingIOS and IntentAndroid in a followup PR, will need to update internal fb code with that.

I don't mind the name Linking - it's used to open external "links".

Looks like you need to rebase, sorry for the delay. Have many PRs in the queue and doing the branch cut for 0.17.

mkonicek · 2015-12-09T22:02:58Z

Adding a label so I don't forget about this PR.

chirag04 · 2015-12-10T17:44:56Z

@mkonicek I'm on vacation till 26th Dec. I will try to rebase and submit the follow up PR soon. Keeping it open till then.

mkonicek · 2015-12-13T14:35:53Z

No problem, enjoy your vacation @chirag04 :)

facebook-github-bot · 2015-12-30T15:18:21Z

@chirag04 updated the pull request.

eslint-bot · 2015-12-30T15:24:36Z

property Linking Property not found in Object.create

Is the bot linting against my PR branch or master?

facebook-github-bot · 2015-12-30T15:31:14Z

@chirag04 updated the pull request.

chirag04 · 2015-12-30T15:33:56Z

@mkonicek This should be go to go now. let me know we need any modification. Happy to submit subsequent PR to remove other files once we get this merged.

eslint-bot · 2015-12-30T15:36:49Z

property Linking Property not found in Object.create

facebook-github-bot · 2015-12-30T15:56:19Z

@chirag04 updated the pull request.

satya164 · 2015-12-30T18:13:43Z

@chirag04IntentAndroid doesn't have a popInitialURL method. It has a getInitialURL method which accepts a callback.

satvikmaker · 2015-12-30T18:31:47Z

How about the name LinkTo sound?

Link to open URL
Link to open app

to here is more like referring to something.

chirag04 · 2015-12-30T19:26:37Z

@satya164 Seems like it was added recently. Thoughts on how we can support both the platforms?

cc @mkonicek

satya164 · 2015-12-30T20:06:15Z

@chirag04 May be the cross-paltform one can have the callback based getInitialURl as both scenarios can be supported by this. Android cannot have a synchronous method because of reasons outlined here - #4320

satya164 · 2015-12-30T20:07:13Z

@bozzmob Not a fan of it. There are method like getInitialURL and addEventListener, which don't sound quite right.

satvikmaker · 2015-12-31T05:45:28Z

@satya164 Agree with you.

mkonicek · 2016-01-06T13:34:34Z

I'm not familiar with how the initial URL works on Android. @satya164, @chirag04 you have more knowledge, feel free to decide on a solution and ship this.

mkonicek · 2016-01-06T13:37:24Z

Looks like it's a constant on iOS and a method on Android? Don't have a strong preference - if it's hard to make it a constant on Android can we just make it a function with a callback in Linking, returning the constant on iOS and calling the method on Android? Up to you.

nicklockwood · 2016-01-06T13:43:58Z

Couldn't you implement popInitialURL on Android by calling getInitialURL internally and then popping to it?

satya164 · 2016-01-06T14:39:45Z

@nicklockwood We could. But it can change, and having it in a constant will be confusing.

mkonicek · 2016-01-06T15:06:26Z

Yeah, I'm not sure when to call getInitialURL. Having a function instead of a constant might be easier?

chirag04 · 2016-01-15T15:03:07Z

closing in favor of #5336. Let's continue the discussion there.

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Dec 3, 2015

mkonicek added the GH Review: accepted label Dec 9, 2015

skevy added Size/L Platform: iOS iOS applications. and removed GH Review: review-needed labels Dec 12, 2015

ide mentioned this pull request Dec 24, 2015
[LinkingIOS] Improve deep linking payload sent to JS on openURL #4955
Closed

chirag04 force-pushed the linking branch from a1eba89 to a4d18e9Compare December 30, 2015 15:17

eslint-bot reviewed Dec 30, 2015
View reviewed changes

chirag04 force-pushed the linking branch from a4d18e9 to afa52a4Compare December 30, 2015 15:30

eslint-bot reviewed Dec 30, 2015
View reviewed changes

Comment threadExamples/UIExplorer/XHRExample.ios.js
eslint-botDec 30, 2015
Copy link
Copy Markdown
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
property Linking Property not found in Object.create

universal linking commponent
fb896ab

chirag04 force-pushed the linking branch from afa52a4 to fb896abCompare December 30, 2015 15:56

chirag04 closed this Jan 15, 2016

Uh oh!

Conversation

chirag04 commented Dec 3, 2015

Uh oh!

facebook-github-bot commented Dec 3, 2015

Uh oh!

vjeux commented Dec 3, 2015

Uh oh!

philikon commented Dec 3, 2015

Uh oh!

satya164 commented Dec 4, 2015

Uh oh!

satya164 commented Dec 4, 2015

Uh oh!

nicklockwood commented Dec 4, 2015

Uh oh!

satya164 commented Dec 4, 2015

Uh oh!

nicklockwood commented Dec 4, 2015

Uh oh!

satya164 commented Dec 4, 2015

Uh oh!

chirag04 commented Dec 4, 2015

Uh oh!

satya164 commented Dec 4, 2015

Uh oh!

chirag04 commented Dec 4, 2015

Uh oh!

mkonicek commented Dec 9, 2015

Uh oh!

mkonicek commented Dec 9, 2015

Uh oh!

chirag04 commented Dec 10, 2015

Uh oh!

mkonicek commented Dec 13, 2015

Uh oh!

facebook-github-bot commented Dec 30, 2015

Uh oh!

eslint-botDec 30, 2015

Choose a reason for hiding this comment

Uh oh!

chirag04Dec 30, 2015

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Dec 30, 2015

Uh oh!

chirag04 commented Dec 30, 2015

Uh oh!

eslint-botDec 30, 2015

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Dec 30, 2015

Uh oh!

satya164 commented Dec 30, 2015

Uh oh!

satvikmaker commented Dec 30, 2015

Uh oh!

chirag04 commented Dec 30, 2015

Uh oh!

satya164 commented Dec 30, 2015

Uh oh!

satya164 commented Dec 30, 2015

Uh oh!

satvikmaker commented Dec 31, 2015

Uh oh!

mkonicek commented Jan 6, 2016

Uh oh!

mkonicek commented Jan 6, 2016

Uh oh!

nicklockwood commented Jan 6, 2016

Uh oh!

satya164 commented Jan 6, 2016

Uh oh!

mkonicek commented Jan 6, 2016

Uh oh!

chirag04 commented Jan 15, 2016

Uh oh!

Reviewers

Assignees

Labels