-
Notifications
You must be signed in to change notification settings - Fork 413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open in desktop #6667
Open in desktop #6667
Conversation
I like the intent of this but we're trying to keep Odysee and LBRY Desktop strongly separated. Please discuss with me over chat. |
Odysee apps can know about Odysee other apps. LBRY cannot know about Odysee. Odysee can know about LBRY URLs, so this may be a way to facilitate Odysee -> LBRY. |
I'll remove the 'Open in web' button. |
@kauffj When I open an external 'lbry://' link on the browser, the desktop app doesn't resolve it. This PR also fixes that bug. |
Changelog update: #6735 |
Can someone test or review this ? |
Thanks for your contribution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks fine but I still have concerns about introducing this element on Odysee, where many users will not have LBRY Desktop.
What's the UX like when the user does not have the desktop app installed? Is there a way we can explain to those users what this feature does?
I think is ignored if there is no protocol handler registered. |
Here are some initial ideas:
|
A similar approach to spotify "Open in desktop" feature will be:
It will probably require a custom route or param for
|
This requires more additional work, I removed all the UI changes #6779 |
@snakyjake1 Did you try https://github.com/lbryio/lbry-desktop/releases/tag/untagged-aa3ae8f9162b42aace2a ? |
@btzr-io I couldn't get your link to work. |
@snakyjake1 Whats your current desktop version ? Where Test 1A, Test 1B run using v0.51.0-rc.2 ? |
PR Checklist
Please check all that apply to this PR using "x":
PR Type
What kind of change does this PR introduce?
Fixes
Issue Number: #6703, #6659
Changelog update: #6735
Web -> Desktop