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.

Sign up for GitHub

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

Jump to bottom

Get featuredImage from assets/ dir or page bundles #759

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Get featuredImage from assets/ dir or page bundles #759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b71591b
Get featuredImage from assets/ dir or page bundles
Tempystral Jan 7, 2023
58ff460
Add tempystral to contributors
Tempystral Jan 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,4 @@
- [Brian Lachniet](https://github.com/blachniet)
- [ShortArrow](https://github.com/ShortArrow)
- [Martin Hellspong](https://github.com/marhel)
- [Tempystral](https://github.com/tempystral)
2 changes: 1 addition & 1 deletion layouts/posts/single.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ <h1 class="title">

<div class="post-content">
{{ if .Params.featuredImage }}
<img src="{{ .Params.featuredImage | relURL }}" alt="Featured image"/>
<img src="{{ or (.Resources.GetMatch .Params.featuredImage) (resources.Get .Params.featuredImage).RelPermalink (.Params.featuredImage | relURL) }}" alt="Featured image"/>
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm... I don't know. This looks like overly complicated. GetMatch could be a problem, because of its pattern matching it could match with a similarly named file. And, resources.Get will try to find the image inside the assets folder, which is not related.

I see two possible solutions for your problem.

  1. Just removing relURL from the original line, so you could use relative path to the page and not to the base URL. (It makes sense, but we'll need to remove relUr from the favicon and avatar code too.)
  2. Use a full path in featureImage such as posts/page-bundle/images/something.jpg.

I don't mind going with the option (1), but people will complain about the breaking change.

Copy link
Author

@Tempystral Tempystral Jan 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I suppose I could use a full path in the featured image, it would just be nice to be able to use a relative path like you can with page resources. I definitely don't want to introduce a breaking change.

As for too complicated... I don't know, I feel like it's simple but the inlining makes it look ugly, perhaps. The goal is to enable the option for bundled images with a relative path, and whatever evaluates truthy first is picked.

That said, I've done some more testing and realized that if I define a page resource with the same path, the image doesn't render correctly, so I suppose I can see the issue there.

I think I will just use an absolute path, or override the single.html template with my own.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, currently this is a practical option. But let me better check other solution this weekend. I think we could create a custom template to render image links and abstract this code. I'm also thinking that using the asset pipeline for images could be a good idea, but I need to understand it better first.

Copy link
Author

Choose a reason for hiding this comment

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

Sure thing. I'm relatively new to using the asset pipeline as well. If you'd like I can draft something in this PR or we can close it and I'll let you do your thing.

{{ end }}
{{ .Content }}
</div>
Expand Down