-
Notifications
You must be signed in to change notification settings - Fork 189
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
Make Youtube link clickable #2706
Conversation
This doesn't solve the issue. What #2679 is about is taking the raw text in a YouTube description, and parsing it to find any URLs, then turning that into HTML that wraps them in There are some existing React components for doing this, e.g., https://www.npmjs.com/package/react-linkify. |
@AmasiaNalbandian for the update I still working on it right now |
Thinking about this more, we should probably use a JS lib vs. React Component for this, maybe https://www.npmjs.com/package/linkifyjs. |
@humphd I suppose to test if this we will only need to run |
I'm not sure what you're doing, I need more context. |
After wrapping the description with telescope/src/web/src/components/Posts/Post.tsx Lines 403 to 407 in 85eb481
|
As I mentioned, don't use a React component for this. Use a JS library that does it. It can happen in the front-end (fine) or back-end (ideal). |
For example, https://www.npmjs.com/package/linkify-it |
src/backend/data/post.js
Outdated
@@ -148,18 +148,26 @@ class Post { | |||
article.date = article.pubdate; | |||
} | |||
|
|||
if (Array.isArray(article.content) && determinePostType(article.link) === 'video') { |
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.
Why are we adding this? When is the content an Array vs. a string, and why have we never hit this in the past (e.g., with @dbelokon's previous work on YouTube)?
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.
When I did debugging all the of the Youtube post return article.content
as an array off object instead of a string like what we expected which make the html
content of all the related video
post not being proccesed
telescope/src/backend/utils/html/index.js
Lines 20 to 21 in 4b2de81
if (typeof html !== 'string') { | |
return html; |
I do not mind, but the deadline for 2.6 is till midnight. Let's ask David about this tomorrow. |
This doesn't depend on microservices. We just need a couple of tests that include a post with text that doesn't have I'm not sure what the question/hold-up is? Let me know. |
@humphd I did add the test and also I want to help moving my code to the |
I think let's not change the |
@TueeNguyen could you do ma a favor and ping me when you move it to the |
@tpmai22 https://github.com/TueeNguyen/telescope/tree/feed-parser |
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.
Can you rebase to add the bug fix #2795? So that we can check if PR passes tests
- Initial work from @manekenpix - Edited `npm run dev` - Export Redis constructor instead - Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581 - Bumped node-fetch to 2.6.7
- Initial work from @manekenpix - Edited `npm run dev` - Export Redis constructor instead - Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581 - Bumped node-fetch to 2.6.7
- Initial work from @manekenpix - Edited `npm run dev` - Export Redis constructor instead - Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581 - Bumped node-fetch to 2.6.7
- Initial work from @manekenpix - Edited `npm run dev` - Export Redis constructor instead - Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581 - Bumped node-fetch to 2.6.7
- Initial work from @manekenpix - Edited `npm run dev` - Export Redis constructor instead - Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581 - Bumped node-fetch to 2.6.7
- Initial work from @manekenpix - Edited `npm run dev` - Export Redis constructor instead - Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581 - Bumped node-fetch to 2.6.7
- Initial work from @manekenpix - Edited `npm run dev` - Export Redis constructor instead - Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581 - Bumped node-fetch to 2.6.7
- Initial work from @manekenpix - Edited `npm run dev` - Export Redis constructor instead - Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581 - Bumped node-fetch to 2.6.7
Issue This PR Addresses
Fixes #2679
Type of Change
Description
Make Youtube link clickable by wrapping
<a>
tag .Checklist
Testing
At the
root
directory removeredis-data
Rebuild images and wait for
redis
to cache the feeds