Skip to content
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

Feature request: generate error when unable to load thumbnail for citations #230

Closed
1 task done
mcourtot opened this issue Jan 5, 2024 · 5 comments · Fixed by #244
Closed
1 task done

Feature request: generate error when unable to load thumbnail for citations #230

mcourtot opened this issue Jan 5, 2024 · 5 comments · Fixed by #244
Labels
enhancement New feature or request

Comments

@mcourtot
Copy link

mcourtot commented Jan 5, 2024

Checks

Link to your website repo

local changes atm

Version of Lab Website Template you are using

1.1.6

Description

When editing the sources.yaml file to add a thumbnail to a citation. If the link to the picture is not correct, no error is generated.

For example, I had mistakenly used
https://github.com/EBISPOT/DUO/blob/master/doc/figs/DUO_logo_white_background.png
instead of
https://raw.githubusercontent.com/EBISPOT/DUO/master/doc/figs/DUO_logo_white_background.png

As a result the thumbnail was not rendered

@vincerubinetti vincerubinetti added the enhancement New feature or request label Jan 5, 2024
@vincerubinetti
Copy link
Collaborator

vincerubinetti commented Jan 5, 2024

For what it's worth, I had this in the documentation at one point, because other people fell into the same problem:

https://github.com/greenelab/lab-website-template/wiki/Tips (legacy docs)

But I guess I didn't include that in the new docs.

This might be slightly difficult/error prone to do. For every source, I'd have to do an actual network request to the url (if it's an absolute url) and also check if it's the right mime type or something (which I believe can be error prone).

Also, for people with giant lists of sources (thinking about the Greene Lab), having it be an actual critical error might be a pain. It would make the entire site build fail if any of the external image links -- which could be from years ago -- suddenly become unavailable (or even temporarily unavailable, if the third party's server goes down). Instead, it'd probably have to be a warning, which unfortunately you'd have to go into the GitHub Actions run log to see. GitHub doesn't have a way (yet) to have a warning status for a PR check (it's only ✅ pass / ❌ fail).

Just want to mention these caveats.

@mcourtot
Copy link
Author

mcourtot commented Jan 5, 2024

Thanks for the feedback - yes didn't see this in the current docs.

I agree checking mime types and failing error seems too much for this.

For what it's worth, the current behavior of not failing (and not displaying anything broken) is IMHO the best. As I am currently building locally via Docker I was thinking of a message in the run logs? For example I have other warnings displayed as
Using Manubot to generate citation## WARNING Generating csl_item for 'eid:2-s2.0-84885705904' failed due to a NotImplementedError: Manubot does not know how to generate a csl_item for 'eid:2-s2.0-84885705904'

When I was trying to load from the wrong path, it didn't display anything wrong (eg no broken image or else), it just keeps the default thumbnail, so maybe there is an opportunity to add an error warning on loading the file and failing?

@vincerubinetti
Copy link
Collaborator

I think I understand what you're asking. The thing is, the errors messages you're referring to are happening at a different level than the image failing to load. For example, that Manubot error is happening at the level of the Python auto-cite process running in Docker, at the CLI level. The image loading gets checked by the browser, only when a user loads the page. It's the browser doing the network request to check the image. So you'd have to somehow communicate from the browser to the command line, and I don't know of any easy way to do that.

Instead, we'd just have to replicate the network request check in the auto-cite script, and that would show up in your console when running locally. I think this is a worthwhile feature to have, because I've seen other people encounter broken images.

But regardless, as you noticed, when an image is not there, any component in the template (citation, portrait, whatever) will gracefully fall back to a nice placeholder image instead of the ugly browser broken image one, so at least we have that.

@vincerubinetti
Copy link
Collaborator

vincerubinetti commented Jan 5, 2024

Actually, I just discovered this Ruby package and its associated Jekyll plugin, which checks for broken links, images, etc:

https://github.com/gjtorikian/html-proofer
https://github.com/episource/jekyll-html-proofer

I'll look into building this into the template, if the output gives is nice and not obtrusive. Not only would this check more things than broken images, presumably it would just run every time you build or even preview the site locally (with live refresh and such).

@vincerubinetti
Copy link
Collaborator

vincerubinetti commented Feb 22, 2024

Good news, I tried out the regular html-proofer gem and it will be added in v1.2.0 of the template. It works very well.

It's not a perfect solution though, there may still be false positives/negatives. But there's nothing we can do about that. It just does a network request and checks the status code returned, and the external website might not return the appropriate one. For example, if you go to this nonsense Zoom room url https://zoom.us/j/abc123, the site returns a 200 code as if everything is fine, when it should really return a 404. The same thing could happen with an image. A link could also be temporarily broken due to some external site being down. As such, as mentioned in comments above, we shouldn't really make an error flagged by html-proofer be a critical error that prevents the site from building, and thus you'll have to spot the logged warnings in the CLI when running locally or in the workflow logs when running on GitHub Actions.

@vincerubinetti vincerubinetti linked a pull request Mar 5, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants