Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

adding code to show thumbnails below tool media carousel (#20) #23

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kots14
Copy link

@kots14 kots14 commented Oct 13, 2020

Issue: analysis-tools-dev/website-next#20
Summary of changes:

  • Added code to fetch thumbnail URLs from YouTube and Vimeo video URLs
  • Added responsive carousel using react-slick.
  • New packages added in the project - react-slick and slick-carousel.

Tested UI output in the following browsers:

  • Mozilla Firefox
  • Google Chrome

src/components/tool/main-media-util-helper.js Outdated Show resolved Hide resolved
parseVideo
} from "./main-media-util-helper"

const getVideoThumbnailUrl = async url => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually I'd love to move the thumbnail creation to build time.
This could be done similarly to the screenshot creation in gatsby-node.js.
The advantage would be that we don't have to run the thumbnail generation on the client.
I think this is a bit outside of the scope of this PR, so let me know if you want to get this in or if we should tackle that at a later point in time.

Copy link
Author

@kots14 kots14 Oct 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mre I'd like to understand whether you want the thumbnails to be downloaded as images or are you referring creation of the thumbnail links at the time of build? Please let me know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we should download the thumbnails as images yes.
Guess you could copy-and-paste this section in the config and replace screenshot with thumbnail.
https://github.com/analysis-tools-dev/website/blob/4d8a0b46e41158fbac61327ced1f6ee49059c21e/gatsby-node.js#L148-L155
Then wherever you need the thumbnail you can use it as a field similar to the screenshot field:
https://github.com/analysis-tools-dev/website/blob/8b82484b73a342618947cf0c48f1340dd11407b3/src/components/tool/main-media.js#L36

Not sure if that's understandable. If not I can add a commit and we can have a look.

Copy link
Author

@kots14 kots14 Nov 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mre Can you please help me out with a npm package that can download thumbnails? I'm trying to work on this actually and so far I was unable to find any. Also, please let me know if I can utilise the code from screenshot.js for downloading thumbnails - I'm trying this as you suggested previously.

Copy link
Member

@mre mre Nov 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please help me out with a npm package that can download thumbnails?

Oh you can totally use your thumbnail code. You could have a thumbnail.js with your code next to screenshot.js and then call that code from gatsby.node.js like above; so something like:

// This could return a dictionary of `{ "<resouce_url>": "<thumbnail_path>" }` or so.
// Alternatively we return `node.resources` with one more field `thumbnail` per entry. 
 const thumbnails = await getThumbnails(node.resources) 
 if (thumbnails) { 
   createNodeField({ 
     node, 
     name: `thumbnails`, 
     value: thumbnails, 
   }) 
 } 

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like I'm in the right path already, I'll get back to you if I'm stuck. Thanks :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mre I've updated the code in this PR. I apologise for the delay.
I tried to put the thumbnail related data under node.resources. However, I couldn't figure it out. Hence, I went with the approach for which you already gave an example. I'm using image-downloader package to download the thumbnails at the build time. I hope that this would sort out all the issues.
Please let me know if further changes are required.

@mre
Copy link
Member

mre commented Oct 25, 2020

For some reason the video thumbnails sometimes don't work for me.
E.g. here is a screenshot of the PVS Studio entry:

image

After a reload or two it starts to work again

image

I'm on Firefox. On Chrome it always works. 🤷

@kots14
Copy link
Author

kots14 commented Oct 26, 2020

Thanks for pointing these out, I'll work on the changes requested.

@kots14
Copy link
Author

kots14 commented Nov 1, 2020

analysis-tools-dev/website-next#23 (comment)

@mre For some reason, I'm unable to reproduce this issue. Tried multiple times with the Firefox browser in my machine. However, it worked all the time. Is there any particular way, I can reproduce this issue?

@mre
Copy link
Member

mre commented Nov 10, 2020

@mre For some reason, I'm unable to reproduce this issue. Tried multiple times with the Firefox browser in my machine. However, it worked all the time. Is there any particular way, I can reproduce this issue?

Huh, just tried again and it now also works on my machine. Odd. Sorry for the fuzz; was probably an issue on my side. 😕

@mre
Copy link
Member

mre commented Nov 10, 2020

Wondering if it was my ad-blocker that was causing that because it was broken in Firefox (where I use uBlock Origin) and not on Chrome where I use... nothing, actually. 💭

@kots14
Copy link
Author

kots14 commented Dec 17, 2020

@mre
Just to check, do I have to update anything in the PR? Please let me know.
Update: Any updates on this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants