Add open.lbry.com deep-links #962

Merged
kekkyojin merged 1 commit from deeplink into master 2020-07-17 12:29:17 +02:00
kekkyojin commented 2020-07-13 19:30:22 +02:00 (Migrated from github.com)

PR Checklist

Please check all that apply to this PR using "x":

  • I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
  • I have checked that this PR does not introduce a breaking change
  • This PR introduces breaking changes and I have provided a detailed explanation below

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting)
  • Refactoring (no functional changes)
  • Documentation changes
  • Other - Please describe:

Fixes

Issue Number: #957

What is the current behavior?

LBRY Android is not offered when user clicks on open.lbry.com links.

What is the new behavior?

LBRY Android will show on lists of apps which can manage open.lbry.com links

Other information

It adds itself for intents with an 'https://" protocol schema and an 'open.lbry.com' host on AndroidManifest.

When system observes an intent from user to visit any link of the form https://open.lbry.com:, it will show a dialog with a list of all apps registered for that intent. When user selects LBRY app, the system will open it and it will recieve the link, which this PR will transform into something like an lbry:// intent, instead of managing it separatelly.

It modifies regex parts as following:

  • Add 'https' as a possible option for protocol part
  • Add a HOST regex part which could only be empty or open.lbry.com

It shifts components for matched strings as a new host component has been added.

In case host part is the expected 'open.lbry.com', it will change separators from ':' to '#'. This is also commented on code.

Note for merger

Maybe this PR could be delayed for next 0.16 release or even a future one as it adds a new feature and not a bug-fix. Until then, I will be attending any change requested by the reviewer.

PR Checklist <!-- For the checkbox formatting to work properly, make sure there are no spaces on either side of the "x" --> Please check all that apply to this PR using "x": - [x] I have checked that this PR is not a duplicate of an existing PR (open, closed or merged) - [x] I have checked that this PR does not introduce a breaking change - [ ] This PR introduces breaking changes and I have provided a detailed explanation below ## PR Type What kind of change does this PR introduce? - [ ] Bugfix - [x] Feature - [ ] Code style update (formatting) - [ ] Refactoring (no functional changes) - [ ] Documentation changes - [ ] Other - Please describe: ## Fixes Issue Number: #957 ## What is the current behavior? LBRY Android is not offered when user clicks on open.lbry.com links. ## What is the new behavior? LBRY Android will show on lists of apps which can manage open.lbry.com links ## Other information It adds itself for intents with an '`https://`" protocol schema and an '`open.lbry.com`' host on AndroidManifest. When system observes an intent from user to visit any link of the form `https://open.lbry.com:`, it will show a dialog with a list of all apps registered for that intent. When user selects LBRY app, the system will open it and it will recieve the link, which this PR will transform into something like an lbry:// intent, instead of managing it separatelly. It modifies regex parts as following: * Add 'https' as a possible option for protocol part * Add a HOST regex part which could only be empty or open.lbry.com It shifts components for matched strings as a new host component has been added. In case host part is the expected 'open.lbry.com', it will change separators from ':' to '#'. This is also commented on code. ### Note for merger Maybe this PR could be delayed for next 0.16 release or even a future one as it adds a new feature and not a bug-fix. Until then, I will be attending any change requested by the reviewer. <!-- If this PR contains a breaking change, please describe the impact and solution strategy for existing applications below. -->
kauffj commented 2020-07-13 20:41:31 +02:00 (Migrated from github.com)

Sweet!! 😁

Sweet!! :grin:
tzarebczan commented 2020-07-14 14:41:33 +02:00 (Migrated from github.com)

This is awesome, thanks so much! We'll get it reviewed soon. Can we show you some appreciation for the contribution?

This is awesome, thanks so much! We'll get it reviewed soon. Can we show you some [appreciation](https://lbry.com/faq/appreciation) for the contribution?
kekkyojin commented 2020-07-14 15:04:27 +02:00 (Migrated from github.com)

Not at this time, but thank you anyway.

Not at this time, but thank you anyway.
akinwale (Migrated from github.com) approved these changes 2020-07-17 12:29:03 +02:00
akinwale (Migrated from github.com) left a comment

Hey, this is great, thanks! I was thinking of also doing this with lbry.tv links, but I felt it may annoy some people who just want to be able to quickly access the web-only experience.

Hey, this is great, thanks! I was thinking of also doing this with lbry.tv links, but I felt it may annoy some people who just want to be able to quickly access the web-only experience.
kekkyojin commented 2020-07-17 18:26:20 +02:00 (Migrated from github.com)

With deep links, user has always the choice about which app to use. If lbry.tv was added, user would also be offered a dialog box and then it could select a web browser or LBRY. But, yes, I also think that lbry.tv could better be left to the web browser experience. At the end, creators will choose one or another, and I think they will be choosing the lbry.tv one.

With deep links, user has always the choice about which app to use. If lbry.tv was added, user would also be offered a dialog box and then it could select a web browser or LBRY. But, yes, I also think that lbry.tv could better be left to the web browser experience. At the end, creators will choose one or another, and I think they will be choosing the lbry.tv one.
tzarebczan commented 2020-07-22 19:01:53 +02:00 (Migrated from github.com)

Yea,I think we should support lbry.tv as well. Many people share those links and would still expect to open in app if available.

Yea,I think we should support lbry.tv as well. Many people share those links and would still expect to open in app if available.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-android#962
No description provided.