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

blog: Use standard summaryLength or explicit summary length (if set) #44

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Staudey
Copy link
Member

@Staudey Staudey commented Oct 2, 2024

This removes some custom overrides of the summary length for blog posts and instead relies on the summaryLength hugo parameter where it's not manually set, and uses the manually set summary length set in single blog posts (set via <!--more-->).

Also allow paragraph html elements in the summary

25 seemed a good value for the default summary length in my testing

Resolves #41

@Staudey
Copy link
Member Author

Staudey commented Oct 3, 2024

Comparison:

Automatic summary before:

Automatic summary after:

Manually set summary before (=ignored):

Manually set summary after:

@Staudey
Copy link
Member Author

Staudey commented Oct 3, 2024

Note: I could also add a conditional to check whether the summary is truncated or contains the whole post content, and only if it is truncated render the "Read More" button, but I'm not sure if that wouldn't be overkill, since it would have to be a very short blog post to fit into it as a whole.

Copy link
Member

@EbonJaeger EbonJaeger left a comment

Choose a reason for hiding this comment

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

One thing, otherwise, LGTM.

{{ end }}
<div class="menu">
<nav>
<div itemprop="description">{{- safeHTML $firstBlog.Summary -}}</div>
Copy link
Member

Choose a reason for hiding this comment

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

Turning this element into a div and removing the unescape/plainify makes summaries with formatting (e.g., italics) display weird. This appears to work:

Suggested change
<div itemprop="description">{{- safeHTML $firstBlog.Summary -}}</div>
<p itemprop="description">{{- htmlUnescape (plainify (safeHTML $firstBlog.Summary)) -}}</p>

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, thanks, I see the problem with italics now after testing. Unfortunately this also loses paragraph separation for long summaries like in the current Hacktoberfest one. (I think that's why I originally turned it into a <div>)

I'll play around with it a bit more.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about:

Suggested change
<div itemprop="description">{{- safeHTML $firstBlog.Summary -}}</div>
{{ $summaryParagraphs := split $firstBlog.Summary "<p>" }}
{{range $summaryParagraphs }}
{{ if eq . "" }}
{{ continue }}
{{ end }}
<p itemprop="description">{{ htmlUnescape (plainify (safeHTML .)) }}</p>
{{end}}

This keeps the paragraphs separated, while escaping/removing other formatting

Copy link
Member Author

Choose a reason for hiding this comment

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

Wow, that formatting looks ass here ^^

Copy link
Member

Choose a reason for hiding this comment

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

I think the formatting is because of tabs vs spaces xD

That said, I'm not entirely sure what that snippet is doing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Splitting the summary along paragraphs, then inserting those paragraphs as separate elements, but with all formatting inside of them plainified.

(and that one if is there to exclude that one empty element of the slice this split creates; maybe I'll find a more elegant solution)

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.

The 'more' tag appears to not work
2 participants