-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add dynamic date on blog pages #5801
Conversation
✅ Deploy Preview for knative ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
@Cali0707 Sure, I'd already tried those environment variables and they work. Which one should I use? The creation date or the revision date? Is it okay to leave the two labels below when the date is already specified after the blog post title? |
Hey @prushh , thanks for the PR! Agreed with what @Cali0707 has mentioned. It would be better to put the date as The ideal condition would be something like this: |
Hi @Leo6Leo, should the ideal condition be the same for all sections (Releases, Articles, etc.)? |
@prushh Yes, let's make all of them have the dates |
Hey @Cali0707 @Leo6Leo, Due to the title that is included in the Another option should be add the following string by hand on each blog post, and have an empty source-file.html file to avoid bottom icons:
I think we can change the style in both cases. |
Personally I think that this looks better, and as we are already adding the blog post dates manually I don't think it's a big ask for new blog posts to include those variables. WDYT @Leo6Leo (also cc @mmejia02 @zainabhusain227 as our UX design leads) |
I prefer the second option too, but this could lead to inconsistencies between old and new posts if occasionally overlooked. If it's not feasible to automate this process by modifying To ensure consistency, we can create a standard blog post template with these variables and integrate a CI test to check for their inclusion in new posts, and implement a system of reminders for content creators, possibly through documentation or a checklist, emphasizing the importance of including these variables. This should help maintain uniformity across our content! |
That's awesome to hear! I think it is okay if the solution isn't too pretty, as long as it can work perfectly. As the Knative UX WG are currently planning to redesign the whole website, so the infrastructure of the website might need to be changed in the future. @Cali0707 WDYT? |
@prushh I like the screenshot of what you have there. Do you think you could push another commit to this PR with the changes so that we can take a closer look at if it is too hacky or not? |
show hack using custom theme
Any suggestions would be appreciated, for now I left the styling inside the overrides/partials/content.html file. I noticed issues when the title is defined inside an Also in Steering Committee section the posts have problems: there isn't any title because in there I should use In Announcing Eventing RabbitMQ moving to GA article there are two |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And for the issue with this RabbitMQ article you mentioned (have 2 h1 tags), I am guessing the issue might be there are 2 # tags in the article? But not sure.[Link]
@@ -0,0 +1,14 @@ | |||
{% if page.meta.git_creation_date_localized and page.meta.git_revision_date_localized %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick suggestion: Let's only show 'Revised on' for posts that have been updated. If it's not edited since posting, we'll skip this part to keep things clear and simple. How does that sound?
</style> | ||
|
||
|
||
{% if page.toc %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think this approach is okay for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where should I move the styling? Is it okay to create the new /overrides/assets/stylesheets/content.css file to avoid problems with docs website?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you think that's a good idea and can explain the reason why this change is worthy, why not? :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think separating CSS from the HTML template is a good practice, as is separating CSS rules based on the files in which they act. For this reason, I will create the file /overrides/assets/stylesheets/content.css that is used only by the /overrides/partials/content.html file and doesn't interact with the docs website 😄 I will also remove the date from blog posts in which it is featured!
[Note] And just a note, this PR relates to the UI change in the knative.dev website, will need approval from Knative Steering Committee. /cc @knative/steering-committee |
/approve I think minor changes like this should be up to the approvers for the |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: evankanderson, prushh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/unhold Thanks for all of your work on this @prushh 🎉 !! |
@prushh it looks like only the |
@Cali0707 I followed Leo's suggestion, do you think is better to leave both? In some posts (e.g. Announcing Knative 1.11 Release, TOC 2022 election announcement, etc.) where no changes have been made the |
@prushh Thanks for the PR and great suggestions you have proposed! Just to clarify my comment: I mean we should always show For example: Article 2 is created on 2024-1-1, but it hasn't been modified since created I hope this is clear! Lmk if there are anything confusing still |
Show always publication date and when a file has been updated, the revision date as well
@Leo6Leo My fault, sorry for the misunderstanding! |
@Cali0707: GitHub didn't allow me to request PR reviews from the following users: review, for, a, final. Note that only knative members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/lgtm This looks good! @prushh Thank you for the contribution! Hope you have learnt something here and we are looking forward to seeing more contribution from you in the knative community Davide! |
* fix NavPluginOrder warning on docs local preview * show non-hardcoded date on blog posts * exclude date from blog homepage * add dates to all blog post show hack using custom theme * fix titles on steering committee section * show only revised or published date, remove styling from template * delete hard-coded dates, fix RabbitMQ article * link content.css stylesheet * fixup on revised and published date Show always publication date and when a file has been updated, the revision date as well
Fixes #5704
Proposed Changes
NavPluginOrder
warning on docs local preview using native Python mkdocs CLImkdocs-git-revision-date-localized-plugin
to add latest update date and creation date labels dynamicallyAdditional Info