-
-
Notifications
You must be signed in to change notification settings - Fork 65
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: add last updated date to blog posts #692
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for pt-br-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ja-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for es-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for de-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for zh-hans-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for fr-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for hi-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for new-eslint ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks for taking a look at this.
Can you move the updated date/time to the left column? It doesn't look very good as another line at the top.
@@ -22,6 +22,7 @@ const path = require("node:path"); | |||
const fs = require("node:fs"); | |||
const yaml = require("js-yaml"); | |||
const { DateTime } = require("luxon"); | |||
const { execSync } = require("node:child_process"); |
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.
Is there not some way built into Eleventy to get the updated date/time? This feels a bit heavy to me.
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.
Yes, that looks good. 👍 |
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.
there is no border between the author's detail and last updated date section for small screen size can you fix that?
Also proposal in #612 says
It would enhance the user experience if the last updated date of the blog post were displayed somewhere on the page, allowing users to quickly see when it was last updated, rather than having to scroll down to the bottom of the page to check the update history.
and for small screen sizes we still have to scroll down to the bottom of the page, should we also add this date in header for small screens?
Thanks for the reviews! I appreciate the feedback. I'm happy with the current design and will wait for Nicholas's input before making any changes. |
@Tanujkanti4441 no, I don't think that necessary. @Pixel998 I like @amareshsm's design. He's on our website team so he's a good resource to work with. |
Prerequisites checklist
What is the purpose of this pull request?
The purpose of the pull request is to add a "last updated" date to blog posts.
What changes did you make? (Give an overview)
I added a "last updated" date to each blog post.
Related Issues
Fix #612
Is there anything you'd like reviewers to focus on?