Handle missing dateOfLaunch by displaying dash placeholder [Fixes #16305]#16443
Conversation
clacladev
requested review from
corwintines,
minimalsm,
pettinarip and
wackerow
as code owners
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| {app.dateOfLaunch | ||
| ? new Date(app.dateOfLaunch).getFullYear() | ||
| : "—"} |
There was a problem hiding this comment.
This looks good overall... slight request though to make this a touch more robust:
We have an isValidDate helper function inside src/lib/utils/date.ts which would not only check if something is "falsy" like this solution, but also makes sure that any string/number we pass to the Date constructor will not evaluate to NaN.
This function is already being used within this file, so it's already imported and ready for use. Would suggest a pattern similar to the getTimeAgo function above on lines 120-121.
const getTimeAgo = (dateString: string) => {
if (!dateString || !isValidDate(dateString)) return "—"So we could add something like this immediately below that function:
const getDisplayYear = (dateString: string) => {
if (!isValidDate(dateString)) return "—"
return new Date(app.dateOfLaunch).getFullYear()
}and then update the lines here with:
| {app.dateOfLaunch | |
| ? new Date(app.dateOfLaunch).getFullYear() | |
| : "—"} | |
| {getDisplayYear(app.dateOfLaunch)} |
(Side note, we wouldn't need the !dateString || check shown in getTimeAgo since TypeScript constraints force that this must be a string, and an empty string passed to isValidDate("") will properly evaluate to false)
There was a problem hiding this comment.
@wackerow you're absolutely right. I should have read better the code to see there was a utility function and a pattern already established.
Anyway, I implemented the suggested changes and it still works as intended. Thanks for the guidance, next time I try to waste less of your time ;)
There was a problem hiding this comment.
Hah didn't waste anyones time, happy to help, thanks @clacladev!
|
@all-contributors please add @clacladev for bug fix |
|
I've put up a pull request to add @clacladev! 🎉 |
|
Happy to help @wackerow |
2 participants
Description
Many Apps in the App Explorer are missing the Founding date, but no check is done, so it's manipulated and displayed as
NaN.Apps where I can reproduce the issue:
https://ethereum.org/apps/eternal-ai/
https://ethereum.org/apps/virtuals/
https://ethereum.org/apps/human-passport/
https://ethereum.org/apps/zk-open-passport/
https://ethereum.org/apps/zk-open-passport/
Solution
I am going to treat the missing Founding Data the same way Last Update Data is treated: displaying a
-(dash) as a placeholder.## Problem and Solution Demo
Before:
After:
## How to test
Related Issue
Issue: #16305 (comment)