-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
Add metadata card #5889
Add metadata card #5889
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5889 +/- ##
==========================================
- Coverage 28.96% 28.83% -0.13%
==========================================
Files 271 270 -1
Lines 23665 23733 +68
Branches 3438 3451 +13
==========================================
- Hits 6854 6843 -11
- Misses 16538 16621 +83
+ Partials 273 269 -4
Continue to review full report at Codecov.
|
@zoek1 is this ready for review? If not, make sure to use the Draft PR functionality when setting up your PR - thanks! |
@danlipert it's ready! 🤓 |
Change medium links to redirect to the Gitcoin blog
testing this on my local, looks really really good. |
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.
Just left a comment to add the html also to the new template since the code had changed a little
@@ -29,5 +29,15 @@ | |||
</button> | |||
</div> | |||
</div> | |||
<div id="thumbnail" class="mt-1 mb-2" style="display: none; top: initial; left: initial; background-color: #FAFAFA"> |
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 add this code also to townsquare/templates/index.html ? now as we changed the design I had tested and is the only missing change to this work with the current code.
For my side just that and this is ready to get merge. Good work
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.
left a few comments @zoek1 -> otherwise LGTM 🙌
If we can get these changes ready -> we can get it in asap
let videoId = youtube[1]; | ||
|
||
if (embedded_resource !== youtube[0]) { | ||
var apiKey = 'AIzaSyDi-EFpC2ntx9PnM_-oiJHk5zCY53KdIf0'; // TODO: add youtube API key to query titles |
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.
Could we define this in settings.py and make it such that the user can define it in their .env file
the YOUTUBE_API_KEY
would have to be passed from the backend -> which we can use here
app/assets/v2/js/status.js
Outdated
if (embedded_resource !== youtube[0]) { | ||
var apiKey = 'AIzaSyDi-EFpC2ntx9PnM_-oiJHk5zCY53KdIf0'; // TODO: add youtube API key to query titles | ||
|
||
let getVideoData = fetchData('https://www.googleapis.com/youtube/v3/videos?key=' + apiKey + '&fields=items(snippet(title))&part=snippet&id=' + videoId); |
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.
let
-> const
<p class="small" style="color: #0056b3">{{ row.metadata.resource.provider }}</p> | ||
</div> | ||
</div> | ||
</div> |
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.
^ could we indent this block by 1
<iframe class="col-sm-12" height="315" src="https://www.youtube.com/embed/{{ row.metadata.resource.id }}" | ||
frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" | ||
allowfullscreen></iframe> | ||
</div> |
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.
^ could we indent this block by 1
Great! I just need to sleep and in a couple of hours I'll push the requested changes. |
Thanks @zoek1, lets get this for this week deploy!! going to test giphy integration also |
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.
chill @zoek1 we'll take care of it
thanks for the awesome work
* fix data model * update active work * fix feedback data * fix icons * uncomment 3box * Add metadata card (#5889) * Embed youtube videos from status * Hide thumbanil by default * Fix lint errors * Substitute youtube URL regex * Add metadata extractor * Display embedded content for youtube and other content * Change medium links to redirect to the gitcoin blog * Update status.js * Update activity.html Co-authored-by: Kevin Owocki <[email protected]> Co-authored-by: Solexplorer <[email protected]> Co-authored-by: Aditya Anand M C <[email protected]> Co-authored-by: Octavio Amuchástegui <[email protected]> * fix up icons * add preview template * youtube api key * 3box under alpha * fix preview grid * meta cards with links * break line * special format for new bounty * makes it so that moderators can hide items on the feed with only one flag * fix travis Co-authored-by: think-in-universe <[email protected]> Co-authored-by: Octavio Amuchástegui <[email protected]> Co-authored-by: Miguel Angel Gordián <[email protected]> Co-authored-by: Kevin Owocki <[email protected]> Co-authored-by: Solexplorer <[email protected]>
Description
Refers/Fixes
#5880
#5883 previous PR
Testing