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…
Add table
Add a link
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-embedwith 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
tooltipis 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-wrappercss 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
nms?I don't think this should be moved into
common/. It is nice not to have to passthumbnail. We can just passuriand then select the thumbnail.Removing it is fine with me.
This should not be in
common/. It should takeuriand 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 namefunction tokenizeMention(eat, value, silent) {const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);This should use
regexValidUriandisValidUri()fromlbry-reduxCan 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 namefunction 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 namefunction 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 namefunction 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 namefunction 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 namefunction tokenizeMention(eat, value, silent) {const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);this is are already implemented inside
parseURI:lbryio/lbry-desktop@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
ClaimListItemcomponent (i'm going to rename toClaimPreview) and added the tooltip in just a couple lines.lbryio/lbry-desktop@99b5cff2b4I 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 👍 💯