Extend markdown support for LBRY urls #2521

Merged
btzr-io merged 27 commits from smart-links into master 2019-06-26 07:23:39 +02:00
btzr-io commented 2019-05-30 22:55:26 +02:00 (Migrated from github.com)

WIP

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

  • Automatically create lbry:// hyperlinks in description #2229

Changes

  • Channel mentions.
  • Add channel information on mouseover #1393
  • @channel -> [@channel](lbry://@chanel)
  • @channel/claim -> [@channel/claim](lbry://@channel/claim)
  • lbry://claim#e -> [lbry://claim#e](lbry://claim#e)

Todo

  • Validate channel url
  • Channel mentions ( custom link )
  • Create channel tooltip
  • Implement channel tooltip
  • Auto-embed lbry urls
> # WIP ## 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) - [ ] 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? - [x] Bugfix - [x] Feature - [ ] Code style update (formatting) - [ ] Refactoring (no functional changes) - [ ] Documentation changes - [ ] Other - Please describe: ## Fixes - [x] Automatically create `lbry://` hyperlinks in description #2229 ## Changes - [x] Channel mentions. - [x] Add channel information on mouseover #1393 ### Automatically generated links - `@channel` -> `[@channel](lbry://@chanel)` - `@channel/claim` -> `[@channel/claim](lbry://@channel/claim)` - `lbry://claim#e` -> `[lbry://claim#e](lbry://claim#e)` ## Todo - [x] Validate channel url - [x] Channel mentions ( custom link ) - [x] Create channel tooltip - [x] Implement channel tooltip - [x] Auto-embed lbry urls
neb-b commented 2019-05-30 23:12:09 +02:00 (Migrated from github.com)

This is awesome!

This is awesome!
neb-b commented 2019-06-10 06:17:54 +02:00 (Migrated from github.com)

Hey @btzr-io! Sorry I lost track of this one. Do you have any urls I can test this at?

Hey @btzr-io! Sorry I lost track of this one. Do you have any urls I can test this at?
btzr-io commented 2019-06-10 08:41:27 +02:00 (Migrated from github.com)

@seanyesmunt Here is a claim with examples for all the new features:

lbry://channel-tooltip#336e32722929861f1e8557381ffac66cb93a4e7b

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:

"Get this episode, DRM free, on LBRY (https://lbry.com) at: @Lunduke"
lbry://does-going-full-stallman-isolate-a#f5761639b0416038e0257046cce7b59f7b4a6e42

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)

"The full unedited video can be watched here"
lbry://one#3ae4ed38414e426c29c2bd6aeab7a6ac5da74a98

@seanyesmunt Here is a claim with examples for all the new features: > lbry://channel-tooltip#336e32722929861f1e8557381ffac66cb93a4e7b 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: > "Get this episode, DRM free, on LBRY (https://lbry.com) at: @Lunduke" > lbry://does-going-full-stallman-isolate-a#f5761639b0416038e0257046cce7b59f7b4a6e42 **Auto-embed lbry urls** I think this is really useful for obscured links like [what is this link?](https://github.com) because usually the lbry url don't give us any information about the content ( it can help with `clickbait`) > "The full unedited video can be watched here" > lbry://one#3ae4ed38414e426c29c2bd6aeab7a6ac5da74a98
btzr-io commented 2019-06-10 08:49:20 +02:00 (Migrated from github.com)

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:

<http://no-preview.com> | http://show-preview.com

But this don't work on inline links like this [link'](<url>)

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: > `<http://no-preview.com>` | `http://show-preview.com` But this don't work on inline links like this `[link'](<url>)`
neb-b commented 2019-06-10 20:42:37 +02:00 (Migrated from github.com)

Cool. I'll mess around with those links. Let me know when you would like me to review this PR.

Cool. I'll mess around with those links. Let me know when you would like me to review this PR.
btzr-io commented 2019-06-10 22:30:20 +02:00 (Migrated from github.com)

I found a plugin for custom props, now we can block the auto-embed with this syntax:

[link](lbry://cute-fat-cats){ data-preview=false }

I found a plugin for custom props, now we can block the `auto-embed` with this syntax: > `[link](lbry://cute-fat-cats){ data-preview=false }`
btzr-io commented 2019-06-11 04:42:13 +02:00 (Migrated from github.com)

@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 )

@seanyesmunt Ok, is ready for review: I noticed some weird glitches / issues with the [tooltip](https://github.com/romainberger/react-portal-tooltip), but I'm still not sure if I fixed all of them :upside_down_face: Also let me know if you notice any performance or loading time issues ( can't confirm nothing yet )
btzr-io (Migrated from github.com) reviewed 2019-06-11 04:53:21 +02:00
btzr-io (Migrated from github.com) commented 2019-06-11 04:49:18 +02:00

I'm not sure if this is the best / clean way render only text from markdown ^^

I'm not sure if this is the best / clean way render only text from markdown ^^
btzr-io (Migrated from github.com) reviewed 2019-06-11 05:01:48 +02:00
btzr-io (Migrated from github.com) commented 2019-06-11 05:01:44 +02:00

Can't access to the scss var from here 😛
should I use a css-var?

Can't access to the scss var from here :stuck_out_tongue: should I use a `css-var`?
btzr-io (Migrated from github.com) reviewed 2019-06-11 21:18:22 +02:00
btzr-io (Migrated from github.com) commented 2019-06-11 21:18:21 +02:00

Never mind looks like there is one already 👍

Never mind looks like there is one already :+1:
btzr-io (Migrated from github.com) reviewed 2019-06-11 21:23:05 +02:00
btzr-io (Migrated from github.com) commented 2019-06-11 21:23:01 +02:00

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}

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}`
btzr-io commented 2019-06-12 07:34:06 +02:00 (Migrated from github.com)

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 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 :upside_down_face:
btzr-io (Migrated from github.com) reviewed 2019-06-12 07:38:10 +02:00
btzr-io (Migrated from github.com) commented 2019-06-12 07:38:05 +02:00

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-501127146

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-501127146
neb-b (Migrated from github.com) requested changes 2019-06-12 17:33:32 +02:00
neb-b (Migrated from github.com) left a comment

This looks awesome. A couple of comments, but nothing major.

This looks awesome. A couple of comments, but nothing major.
@ -0,0 +84,4 @@
}
}
export default ClaimLink;
neb-b (Migrated from github.com) commented 2019-06-12 17:21:58 +02:00

This component is very similar to ChannelLink, they should probably just be combined and separated by a prop

This component is very similar to `ChannelLink`, they should probably just be combined and separated by a prop
neb-b (Migrated from github.com) commented 2019-06-12 17:22:58 +02:00

Can the color styles be moved into css?

Can the color styles be moved into css?
neb-b (Migrated from github.com) commented 2019-06-12 17:23:35 +02:00

Is there any way to delay the hover effect? So it won't show until you've hovered for n ms?

Is there any way to delay the hover effect? So it won't show until you've hovered for `n` ms?
neb-b (Migrated from github.com) commented 2019-06-12 17:25:53 +02:00

I don't think this should be moved into common/. It is nice not to have to pass thumbnail. We can just pass uri and then select the thumbnail.

I don't think this should be moved into `common/`. It is nice not to have to pass `thumbnail`. We can just pass `uri` and then select the thumbnail.
neb-b (Migrated from github.com) commented 2019-06-12 17:26:45 +02:00

Removing it is fine with me.

Removing it is fine with me.
neb-b (Migrated from github.com) commented 2019-06-12 17:28:16 +02:00

This should not be in common/. It should take uri and use selectors to get everything else.

This should not be in `common/`. It should take `uri` and use selectors to get everything else.
neb-b (Migrated from github.com) commented 2019-06-12 17:29:01 +02:00

We shouldn't need to do this.

We shouldn't need to do this.
neb-b (Migrated from github.com) commented 2019-06-12 17:29:51 +02:00

These styles should live in tooltip.scss

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);
neb-b (Migrated from github.com) commented 2019-06-12 17:31:49 +02:00

This should use regexValidUri and isValidUri() from lbry-redux

This should use `regexValidUri` and `isValidUri()` from `lbry-redux`
neb-b commented 2019-06-12 17:51:04 +02:00 (Migrated from github.com)

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.

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.
btzr-io (Migrated from github.com) reviewed 2019-06-13 02:47:02 +02:00
btzr-io (Migrated from github.com) commented 2019-06-13 02:47:02 +02:00

There is no easy / clean way to use a css selector: https://github.com/romainberger/react-portal-tooltip/issues/36

There is no easy / clean way to use a css selector: https://github.com/romainberger/react-portal-tooltip/issues/36
btzr-io (Migrated from github.com) reviewed 2019-06-13 02:53:10 +02:00
btzr-io (Migrated from github.com) commented 2019-06-13 02:53:09 +02:00
https://github.com/romainberger/react-portal-tooltip/issues/30
btzr-io (Migrated from github.com) reviewed 2019-06-13 02:54:09 +02:00
@ -0,0 +84,4 @@
}
}
export default ClaimLink;
btzr-io (Migrated from github.com) commented 2019-06-13 02:54:09 +02:00

Should be called ClaimLink ?

Should be called `ClaimLink` ?
btzr-io (Migrated from github.com) reviewed 2019-06-13 05:50:42 +02:00
btzr-io (Migrated from github.com) commented 2019-06-13 05:50:42 +02:00
Not possible `ATM`, see: https://github.com/romainberger/react-portal-tooltip/issues/78 :disappointed:
btzr-io (Migrated from github.com) reviewed 2019-06-13 06:19:56 +02:00
@ -0,0 +84,4 @@
}
}
export default ClaimLink;
btzr-io (Migrated from github.com) commented 2019-06-13 06:19:56 +02:00

Done 👍

Done :+1:
btzr-io (Migrated from github.com) reviewed 2019-06-13 07:21:13 +02:00
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
btzr-io (Migrated from github.com) commented 2019-06-13 07:21:13 +02:00

I can't find regexValidUri, did you mean regexInvalidURI ?
https://github.com/lbryio/lbry-redux/blob/master/src/lbryURI.js#L5

I can't find `regexValidUri`, did you mean `regexInvalidURI` ? https://github.com/lbryio/lbry-redux/blob/master/src/lbryURI.js#L5
btzr-io (Migrated from github.com) reviewed 2019-06-13 08:26:41 +02:00
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
btzr-io (Migrated from github.com) commented 2019-06-13 08:26:41 +02:00
this? https://github.com/lbryio/lbry-redux/blob/master/src/lbryURI.js#L32
btzr-io (Migrated from github.com) reviewed 2019-06-13 08:29:44 +02:00
btzr-io (Migrated from github.com) commented 2019-06-13 08:29:44 +02:00

I can't use any connected components inside the tooltip, I think is because of the portal api ?

I can't use any connected components inside the `tooltip`, I think is because of the portal api ?
btzr-io (Migrated from github.com) reviewed 2019-06-13 08:30:21 +02:00
btzr-io (Migrated from github.com) commented 2019-06-13 08:30:20 +02:00
See: https://github.com/lbryio/lbry-desktop/pull/2521#discussion_r293222831
btzr-io (Migrated from github.com) reviewed 2019-06-13 08:31:00 +02:00
btzr-io (Migrated from github.com) commented 2019-06-13 08:31:00 +02:00

Any ideas on this ?

Any ideas on this ?
neb-b (Migrated from github.com) reviewed 2019-06-17 18:18:52 +02:00
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
neb-b (Migrated from github.com) commented 2019-06-17 18:18:52 +02:00

whoops. yes

whoops. yes
neb-b (Migrated from github.com) reviewed 2019-06-17 18:20:08 +02:00
neb-b (Migrated from github.com) commented 2019-06-17 18:20:07 +02:00

Is this with reach tooltip? I don't think that should matter, but I'm not sure.

Is this with reach tooltip? I don't think that should matter, but I'm not sure.
neb-b (Migrated from github.com) reviewed 2019-06-17 18:20:52 +02:00
neb-b (Migrated from github.com) commented 2019-06-17 18:20:51 +02:00

I think we should try to use reach tooltip then

I think we should try to use reach tooltip then
btzr-io (Migrated from github.com) reviewed 2019-06-18 05:36:28 +02:00
btzr-io (Migrated from github.com) commented 2019-06-18 05:36:28 +02:00

👍

:+1:
btzr-io (Migrated from github.com) reviewed 2019-06-20 23:29:07 +02:00
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
btzr-io (Migrated from github.com) commented 2019-06-20 23:29:07 +02:00

@seanyesmunt not sure how this is done on reach-ui tooltip 🙃

I mean the delay time to show / hide.

@seanyesmunt not sure how this is done on reach-ui tooltip :upside_down_face: I mean the delay time to show / hide.
btzr-io (Migrated from github.com) reviewed 2019-06-21 03:34:55 +02:00
@ -0,0 +40,4 @@
// Generate a markdown link from channel name
function tokenizeMention(eat, value, silent) {
const match = /^@+[a-zA-Z0-9-#:/]+/.exec(value);
btzr-io (Migrated from github.com) commented 2019-06-21 03:34:55 +02:00

this is are already implemented inside parseURI:
b7adc597e2/src/ui/util/remark-lbry.js (L23)

this is are already implemented inside `parseURI`: https://github.com/lbryio/lbry-desktop/blob/b7adc597e232f95dea200a6f49bf5d17c1af102c/src/ui/util/remark-lbry.js#L23
btzr-io commented 2019-06-21 03:35:52 +02:00 (Migrated from github.com)

@seanyesmunt ready for review ✌️

@seanyesmunt ready for review :v:
neb-b (Migrated from github.com) requested changes 2019-06-24 06:16:41 +02:00
neb-b (Migrated from github.com) left a comment

I ended up adding reach/tooltip to the branch I'm working on. I used the ClaimListItem component (i'm going to rename to ClaimPreview) 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.

I ended up adding reach/tooltip to the branch I'm working on. I used the `ClaimListItem` component (i'm going to rename to `ClaimPreview`) and added the tooltip in just a couple lines. https://github.com/lbryio/lbry-desktop/commit/99b5cff2b41fd1592d468b8b5df9e3eb0b3ab894 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.
btzr-io commented 2019-06-24 19:03:41 +02:00 (Migrated from github.com)

@seanyesmunt Ok done 👍

@seanyesmunt Ok done :+1:
btzr-io commented 2019-06-24 19:04:45 +02:00 (Migrated from github.com)

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.

yeah that looks better and clean 👍 💯

> 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. yeah that looks better and clean :+1: :100:
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-desktop#2521
No description provided.