-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
fix(content-docs): format last update date as "Jun 19, 2020" #7673
fix(content-docs): format last update date as "Jun 19, 2020" #7673
Conversation
Hi @sigwinch28! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
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'm inclined to call it a design decision, though I don't have strong opinions (a lot of people have complained over time and I agree it's ambiguous without context). The idea is that most blog posts (e.g. Medium, dev.to) display dates in full form but doc metadata tends to be more succinct. We have #7249 for this. Do you want to implement that one instead?
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
try { | ||
return new Intl.DateTimeFormat(locale, { | ||
day: 'numeric', | ||
month: 'long', |
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.
At any rate, I'd prefer this to be short
, like Jun 19, 2020
. MDN and the TypeScript website both use this style.
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.
Changed.
I have some ideas for that. Should I discuss those ideas in that issue? |
Actually, I like the current design better than updating by user locale, so I'll probably close that one with this anyway. Do you think it would be fine to show "Jun 19, 2020" instead of "19 Jun, 2020" to a UK reader? I think it's at least unambiguous. |
I at least think it's unambiguous in English, because I can't conflate "Jun" with the day, i.e. "the 6th day of the month". If we wanted to make this user-configurable, then we could extend the
That would allow:
|
We don't need an option for this; if you want to format this differently on server-side you can do so with |
In that case, I'm happy without the option, too :) Just waiting for the CLA check to update now. |
In fact the mention of this is in a pretty obscure place: https://docusaurus.io/docs/next/api/docusaurus-config#i18n:~:text=To%20choose%20the%20format%20(DD/MM/YYYY%20or%20MM/DD/YYYY)%2C%20set%20your%20locale%20name%20to%20en%2DGB%20or%20en%2DUS%20(en%20means%20en%2DUS). so not surprised if people don't discover it |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
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.
Just want a second opinion. @slorber WDYT?
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
That change looks fine to me, but will every user agree with this opinionated change? 🤷♂️
I'm not sure how it fixes your own problem, as it now displays a single date. Apart being unambiguous, that doesn't look like the ideal solution. The main problem is that you want to support en-GB + en-US using a single site locale instead of 2 🤪 Apart from "fixing" the date format after hydration, there's nothing we can do. Note I'm not even sure if we still need to format those dates on the Node.js side. #4344 => not much context remains, but it looks like it was related to using the polyfill only in Node.js side and ensure it does not increase client bundle size. Wonder if we should remove that and now do all date formatting on the client instead? Now users would have the responsibility to choose how to format dates. It can use site locale by default (SSR+CSR), and eventually switch to user locale format after hydration on a case-by-case basis. We could have a Note: using a |
Yes, but this single date is unambiguous, and both en-US and en-GB users would be able to comprehend it without even saying "uhh, this is weird". MDN and TypeScript both use this format. |
Yes, I'm fine with this change, just want to highlight it could be better to format locally and provide more swizzle-friendly flexibility to change it (not necessarily with a first-class API) |
Pre-flight checklist
Motivation
I find that the "Last Updated" dates on docs pages are ambiguous using the
en
locale becauseen
is interpreted asen-US
, which createsMM/DD/YYYY
instead ofDD/MM/YYYY
.However, I work on an international team, where North American colleagues expect
MM/DD/YYYY
, but my European colleagues expectDD/MM/YYYY
.Others have also experienced issues with the date format in the docs. See related issues.
So I want an unambiguous date format in the docs
Fortunately, it seems that the blog already uses an unambiguous date format of
Month DD, YYYY
:docusaurus/packages/docusaurus-plugin-content-blog/src/blogUtils.ts
Lines 151 to 168 in 2c7012f
Therefore, to bring an unambiguous date format to the docs, I felt it would be least surprising to copy what the blog package does.
In my own sites the workaround I currently use is to swizzle the (unsafe!)
LastUpdated
component to achieve the same effect.Test Plan
I have updated tests for the doc package which check the exact
lastUpdated
string as rendered.Test links
Deploy preview: https://deploy-preview-7673--docusaurus-2.netlify.app/
Related issues/PRs
This has been brought up before: