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

feat: old content warning #266

Merged
merged 21 commits into from
Nov 11, 2021
Merged

feat: old content warning #266

merged 21 commits into from
Nov 11, 2021

Conversation

ericswpark
Copy link
Contributor

WIP, needs styling on the alert div. Basic functionality works

Closes #250

@ericswpark
Copy link
Contributor Author

Forgot to mention but thanks to https://brunty.me/post/display-message-on-old-hugo-entries/ for the original code.

@lxndrblz @SchabrechtsK please let me know if this implementation is okay and please help me with styling. I'm not sure what will look good. Right now the text is inserted right above the content, without any visual distinction which might look odd.

@lxndrblz
Copy link
Owner

lxndrblz commented Nov 8, 2021

@ericswpark Thanks! I'll take care of the CSS styling.

@ericswpark
Copy link
Contributor Author

I changed the implementation so it now displays the age of the post rather than the duration cutoff, which looks better IMHO at conveying the age of the post. Also moved the string to i18n.

Should we consider allowing users to customize the outdated content message shown? Or should we just display the generalized outdated statement?

@lxndrblz
Copy link
Owner

lxndrblz commented Nov 10, 2021

@ericswpark I would leave it a default message and not customizable.

Personally, I would change the wording to include the value set in the old content duration and emphasis the "over", because the problem of this static approach is that if a user does not rebuild their sites daily, the information would be incorrect.

@lxndrblz
Copy link
Owner

@ericswpark Just pushed some basic CSS styling:

image

What do you think?

@ericswpark
Copy link
Contributor Author

@lxndrblz

Personally, I would change the wording to include the value set in the old content duration and emphasis the "over", because the problem of this static approach is that if a user does not rebuild their sites daily, the information would be incorrect.

I haven't even considered that! I'll change it back to the original wording.

...
image

What do you think?

Looks good. Maybe we can make it yellow-colored to better distinguish it as a warning on the page.

@ericswpark
Copy link
Contributor Author

@lxndrblz here's the alert with the new changes:

image

The "!" indicator wasn't centered vertically in the circle and I couldn't find a way to center it with padding so I just changed the height to center the indicator I totally meant to make it sideways-oval shaped; it looks better right?

Also added fix for redefined post section name.

@lxndrblz
Copy link
Owner

We should also add the remaining translations before this goes live.

@ericswpark
Copy link
Contributor Author

@lxndrblz wouldn't it be better to make it a pinned issue? Translators can't tack on translation commits to this PR unless they are a maintainer of the repository and getting all the translations in will take a lot of time unless we can contact all the previous translators. Since the feature is disabled by default I think translations will trickle in as people enable it and see that there is no translated string.

@lxndrblz lxndrblz self-assigned this Nov 11, 2021
@lxndrblz lxndrblz added the enhancement New feature or request label Nov 11, 2021
@sonarcloud
Copy link

sonarcloud bot commented Nov 11, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Owner

@lxndrblz lxndrblz left a comment

Choose a reason for hiding this comment

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

@ericswpark LGTM! Thanks for all your help.

@lxndrblz lxndrblz merged commit 33fe67a into lxndrblz:master Nov 11, 2021
@ericswpark ericswpark deleted the oldcontent branch November 11, 2021 17:09
@agilix
Copy link
Contributor

agilix commented Nov 14, 2021

Very nice! Thanks for the hard work @ericswpark !
Personal circumstances prevented me from working at all. Great to see it in here 😄

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 this pull request may close these issues.

Mark a post as outdated automatically after a period.
3 participants