Extend markdown support for LBRY urls #2521
No reviewers
Labels
No labels
accessibility
app-parity
area: creator
area: daemon
area: design
area: devops
area: discovery
area: docs
area: installer
area: internal
area: livestream
area: performance
area: proposal
area: reposts
area: rewards
area: search
area: security
area: subscriptions
area: sync
area: ux
area: viewer
area: wallet
BEAMER
channel
comments
community PR
consider soon
core team
css
dependencies
electron
Epic
feature request
first-timers-only
good first issue
hacktoberfest
help wanted
hub-dependent
icebox
Invalid
level: 0
level: 1
level: 2
level: 3
level: 4
merge when green
needs: exploration
needs: grooming
needs: priority
needs: repro
needs: tech design
notifications
odysee
on hold
playlists
priority: blocker
priority: high
priority: low
priority: medium
protocol dependent
recsys
redesign
regression
resilience
sdk dependent
Tom's Wishlist
trending
type: bug
type: discussion
type: improvement
type: new feature
type: refactor
type: task
type: testing
unplanned
windows
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: LBRYCommunity/lbry-desktop#2521
Loading…
Reference in a new issue
No description provided.
Delete branch "smart-links"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
PR Checklist
Please check all that apply to this PR using "x":
PR Type
What kind of change does this PR introduce?
Fixes
lbry://
hyperlinks in description #2229Changes
Automatically generated links
@channel
->[@channel](lbry://@chanel)
@channel/claim
->[@channel/claim](lbry://@channel/claim)
lbry://claim#e
->[lbry://claim#e](lbry://claim#e)
Todo
This is awesome!
Hey @btzr-io! Sorry I lost track of this one. Do you have any urls I can test this at?
@seanyesmunt Here is a claim with examples for all the new features:
But you can see some real examples already working here:
Mentions for channels:
I think this is a good example of this feature but you could also mention a friend's channel or another channel you own:
Auto-embed lbry urls
I think this is really useful for obscured links like what is this link? because usually the lbry url don't give us any information about the content ( it can help with
clickbait
)Also I still need to find a way to let the user choose if the auto-embed feature should apply for a specific link. I think discord has a syntax for this:
But this don't work on inline links like this
[link'](<url>)
Cool. I'll mess around with those links. Let me know when you would like me to review this PR.
I found a plugin for custom props, now we can block the
auto-embed
with this syntax:@seanyesmunt Ok, is ready for review:
I noticed some weird glitches / issues with the tooltip, but I'm still not sure if I fixed all of them 🙃
Also let me know if you notice any performance or loading time issues ( can't confirm nothing yet )
I'm not sure if this is the best / clean way render only text from markdown ^^
Can't access to the scss var from here 😛
should I use a
css-var
?Never mind looks like there is one already 👍
We probably don't need to use the
data-
prefix since it doesn't get passed to the actual DOM, and it will make the syntax more simple to use, what do you think ?[link](url){preview=false}
or[link](url){embed=false}
I think all those glitches / issues are gone, but now there is only one problem when the
tooltip
is positioned outside the visible bounds of the window, it will create a white rectangle area, because there is no overflow on the y axis 🙃I tried to apply overflow for both axis (x / y), but I'll need to change
.main-wrapper
css as well in order to work, see: https://github.com/lbryio/lbry-desktop/pull/2521#issuecomment-501127146This looks awesome. A couple of comments, but nothing major.
@ -0,0 +84,4 @@
}
}
export default ClaimLink;
This component is very similar to
ChannelLink
, they should probably just be combined and separated by a propCan the color styles be moved into css?
Is there any way to delay the hover effect? So it won't show until you've hovered for
n
ms?I don't think this should be moved into
common/
. It is nice not to have to passthumbnail
. We can just passuri
and then select the thumbnail.Removing it is fine with me.
This should not be in
common/
. It should takeuri
and use selectors to get everything else.We shouldn't need to do this.
These styles should live in
tooltip.scss
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
This should use
regexValidUri
andisValidUri()
fromlbry-redux
Can you try to use https://ui.reach.tech/tooltip/?
I was planning to use this for the regular tooltips. It has great keyboard/accessibility support. If it looks like a lot of trouble don't worry about it.
There is no easy / clean way to use a css selector: https://github.com/romainberger/react-portal-tooltip/issues/36
https://github.com/romainberger/react-portal-tooltip/issues/30
@ -0,0 +84,4 @@
}
}
export default ClaimLink;
Should be called
ClaimLink
?Not possible
ATM
, see: https://github.com/romainberger/react-portal-tooltip/issues/78 😞@ -0,0 +84,4 @@
}
}
export default ClaimLink;
Done 👍
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
I can't find
regexValidUri
, did you meanregexInvalidURI
?https://github.com/lbryio/lbry-redux/blob/master/src/lbryURI.js#L5
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
this? https://github.com/lbryio/lbry-redux/blob/master/src/lbryURI.js#L32
I can't use any connected components inside the
tooltip
, I think is because of the portal api ?See: https://github.com/lbryio/lbry-desktop/pull/2521#discussion_r293222831
Any ideas on this ?
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
whoops. yes
Is this with reach tooltip? I don't think that should matter, but I'm not sure.
I think we should try to use reach tooltip then
👍
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
@seanyesmunt not sure how this is done on reach-ui tooltip 🙃
I mean the delay time to show / hide.
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
this is are already implemented inside
parseURI
:b7adc597e2/src/ui/util/remark-lbry.js (L23)
@seanyesmunt ready for review ✌️
I ended up adding reach/tooltip to the branch I'm working on. I used the
ClaimListItem
component (i'm going to rename toClaimPreview
) and added the tooltip in just a couple lines.99b5cff2b4
I really like what you did with your channel tooltip, but to keep development simpler in the future I'm going to just use this ^ for now.
Everything else looks good. I can merge once the tooltip stuff is removed.
@seanyesmunt Ok done 👍
yeah that looks better and clean 👍 💯