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

Article image #83

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

Article image #83

wants to merge 3 commits into from

Conversation

aquatix
Copy link
Contributor

@aquatix aquatix commented Nov 23, 2019

Support showing the image configured in an article in both the header of the article itself as on the index page, below the summary of the article.

An example can be found on my weblog.

@aquatix
Copy link
Contributor Author

aquatix commented Dec 3, 2019

Is this addition something you would consider including? It only shows the image when the Image: header is included in the article file.

I would prefer not to have to override both templates with a complete copy of my own through THEME_TEMPLATES_OVERRIDES :)

@sio
Copy link
Collaborator

sio commented Dec 3, 2019

I like the idea overall but I didn't have the time yet to review the implementation and to think about possible edge cases. Most likely this will get merged eventually :)

I'll try to return to this in the following week.

@aquatix
Copy link
Contributor Author

aquatix commented Dec 3, 2019

No worries, take your time!

@sio
Copy link
Collaborator

sio commented Jun 1, 2021

I'm sorry, life happened and this PR fell off my radar.

Why did you add the last commit (43b5acb)? Why is it beneficial for you to duplicate article image manually at the top of the article body like you do here?

Do you think that may be there should be max-height or min-width set on article image? Now it looks weird if image was not specifically cropped/resized to fit in blogroll:

screenshot

@aquatix
Copy link
Contributor Author

aquatix commented Jun 1, 2021

Hey, no worries :)

I changed the way those article images are 'included' as they would not show up in the rss/atom feed for example. The way those feeds are generated, is pretty direct from the content, so I missed those in the previous situation. It is not ideal to have them duplicated, so I might change it anyway, but that will mean creating a template for feeds, I think?

Adding a max-height and/or min-width sounds like a good idea, so tall images do not take over the page :)

Do you have an idea how to easily include the article image in the feeds? That would solve the issue of the duplication.

@sio
Copy link
Collaborator

sio commented Jun 1, 2021

Do you have an idea how to easily include the article image in the feeds? That would solve the issue of the duplication.

I've just looked it up and it appears that Pelican feeds are not templated but instead are generated as XML trees.

It means we can't easily modify feed output. Simplest solution I see would be to override elements in Writer.write_feed here, but that still requires creating a Pelican plugin which is far out of scope for our simple theme.


Another note

It seems that there is no consensus among theme authors about which attribute to use for article image. You suggest article.image, but there are also article.cover, article.featured_image and even article.picture in use :)


I agree that article.image is a good name for your use case, and it's sensible to expect it to appear in RSS/ATOM feed. With article.cover for example we would not have that problem because that image would be only a styling feature and not part of content - no one would need it in their RSS reader.

I see the following options:

  • Show article.image only in index.html. Do not consider the image to be part of content. Rename it to article.cover maybe?
  • Show article.image both in index.html and in article.html. Accept that the image will not appear in feeds.

What do you think?

@aquatix
Copy link
Contributor Author

aquatix commented Jun 1, 2021

Yeah, those XML trees were what I was talking about, so I quickly gave up on the thought of changing those :)

Article images are indeed far from standardised; today I discovered https://github.com/peterdesmet/pelican-cover-image which does something pretty similar. I like 'our' short 'image:' header.

Those two options are exactly what I was contemplating; currently, I chose the first option, so the article.image shows up in overviews, but not in the article itself. I prefer that over not having the image show up in feeds, as sometimes it is a big part of the content of that post.

I do not mind calling it article.image or article.cover as I think both are pretty much the same kind of thing: "featured image for this article". The result is the same. What do you prefer?

@aquatix
Copy link
Contributor Author

aquatix commented Jun 1, 2021

I think I prefer 'Image' by the way, it is a clear, concise title. 'Cover' is slightly more ambiguous. Also, I have a big-ish list of articles with 'Image:' already ;)

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

Successfully merging this pull request may close these issues.

2 participants