search

chipzoller / hugo-clarity

A theme for Hugo based on VMware Clarity
Other
571 stars 263 forks source link

Add pinning featured posts option #353

Closed LukeHong closed 2 years ago

LukeHong commented 2 years ago

This PR...

Changes / fixes

Screenshots (if applicable)

difference

Checklist

Ensure you have checked off the following before submitting your PR.

rootwork commented 2 years ago

Looks like a good feature. Same as the other PR, haven't tested locally yet but approving the test builds.

LukeHong commented 2 years ago

The last push is to fix the variable name style

LukeHong commented 2 years ago

Fix missing signoff issue, should be all good after being approved

rootwork commented 2 years ago

[Edit: Nevermind about what I posted earlier; it was an error on my end.]

I would suggest changing the comment for numberOfPinnedPosts to read something like:

# Maximum number of pinned featured posts (default: 8)

since as I read the code it looks like this param doesn't need to be set in order for the pinning to work (which is good).

Also, somewhat tangentially related to this PR, I noticed that we have a "featured" tag, but that posts are only featured when featured: true is set -- it has nothing to do with the tag. We might want to address this, or at least document it, in a future PR [Edit: Opened an issue for discussion, #356].

rootwork commented 2 years ago

Tested and confirmed that this works; sorry about the false negative I reported in the previous comment.

See my suggestion about the comment; otherwise this looks good.

Thanks for contributing @LukeHong!

LukeHong commented 2 years ago

@rootwork I also add it to the comment for numberOfFeaturedPosts and numberOfRecentPosts which also have a default value in code

rootwork commented 2 years ago

Looks great! And you're right, good catch. Going to merge.

Thanks for this contribution as well @LukeHong!

CyberPunkie commented 11 months ago

Above does not seem to work. Using latest Hugo and Clarity:

  1. featured: true It makes the post appear in "Featured" section, but removes it from "Recent"

  2. featured: false It makes the post appear in "recent" section, but removes it from "Featured"

  3. featured: true + tag:featured It makes the post appear in featured section BUT it is not pinned at the top of the post list.

Am I missing anything?

LukeHong commented 10 months ago

Replied at #451 .