-
Notifications
You must be signed in to change notification settings - Fork 16
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 time tag to the date and change some date format to YYYY-MM-DD #541
Add time tag to the date and change some date format to YYYY-MM-DD #541
Conversation
1faf6f9
to
149d1cf
Compare
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.
Hi @ChengShi-1 , leave some comments on test files and we are missing datetime
attribute on all the <time />
tags.
See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/time
tests/component/sections/file/file-embargo/FileEmbargoDate.spec.tsx
Outdated
Show resolved
Hide resolved
tests/component/sections/file/file-embargo/FileEmbargoDate.spec.tsx
Outdated
Show resolved
Hide resolved
tests/component/sections/file/file-embargo/FileEmbargoDate.spec.tsx
Outdated
Show resolved
Hide resolved
tests/component/sections/file/file-embargo/FileEmbargoDate.spec.tsx
Outdated
Show resolved
Hide resolved
Hi German! Thank you a lot for reviewing it! I submit a couple of fix here. The earliest fix included most of the adjustments, but the dateHelper in utcTime couldn't pass the e2e so I had next two fix commits for recovering the code. Sorry for messed commits, hope you are not confused. |
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.
Some more tweaks for the datetime attribute value
...tions/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/FileDate.tsx
Outdated
Show resolved
Hide resolved
@ChengShi-1 It looks like file uploading is broken with this PR 😫.
I was thinking it might be a local issue but wanted to check with you guys first. Screen.Recording.2024-11-12.at.3.15.20.PM.mov |
Hi, Omer! Sorry, it should be for http://localhost:8000/spa instead of JSF(http://localhost:8000). Can you try |
@ChengShi-1 The file upload is still failing, see recording below:
Screen.Recording.2024-11-13.at.1.53.35.PM.mov |
@ofahimIQSS Hi Omer, I don't meet the same uploading error on my end. Since I don't think this PR changes the code about uploading, could you try upload files in other branch to see if the uploading failure happens globally, and rebuild docker to try upload again as well? Ty! |
No issues found with PR. Merging. |
What this PR does / why we need it:
For solving some existing todos, add time tag to the date so as to provides semantic meaning to the content. The date attribute within the tag allows you to provide a machine-readable format of the date and time, which can be useful for search engine optimization (SEO), calendar integration, and other purposes. Also, in order to keep constant with JSF, we need change some date format to YYYY-MM-DD as well.
Which issue(s) this PR closes:
Special notes for your reviewer:
Suggestions on how to test this:
Step 1: Run the Development Environment
Step 2: check if the date run well and in a right format YYYY-MM-DD
7.under "File Metadata", click the file title, the filed "Deposit Date","Metadata Release Date", and "Publication Date" should be shown in YYYY-MM-DD format.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: