-
-
Notifications
You must be signed in to change notification settings - Fork 13
adding code to show thumbnails below tool media carousel (#20) #23
base: master
Are you sure you want to change the base?
Conversation
parseVideo | ||
} from "./main-media-util-helper" | ||
|
||
const getVideoThumbnailUrl = async url => { |
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.
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.
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.
@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.
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.
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.
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.
@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.
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 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,
})
}
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.
Seems like I'm in the right path already, I'll get back to you if I'm stuck. Thanks :)
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.
@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.
Thanks for pointing these out, I'll work on the changes requested. |
58295db
to
7691ab1
Compare
7691ab1
to
9690be8
Compare
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? |
Huh, just tried again and it now also works on my machine. Odd. Sorry for the fuzz; was probably an issue on my side. 😕 |
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. 💭 |
@mre |
Issue: analysis-tools-dev/website-next#20
Summary of changes:
react-slick
.react-slick
andslick-carousel
.Tested UI output in the following browsers: