-
Notifications
You must be signed in to change notification settings - Fork 5.2k
setup /roadmap for translation #16057
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
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project 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.
Looking good @corwintines! Working as-is, left a couple comments to potentially iterate on
// Legacy export for backward compatibility - uses hardcoded English strings | ||
export const releasesData: Release[] = getReleasesData((key: string) => { | ||
// This is a fallback that returns the key itself if translations aren't available | ||
// In practice, this should not be used in the actual app | ||
console.warn(`Translation key ${key} used without translation function`) | ||
return key | ||
}) |
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.
Do we need this?
const locale = useLocale() | ||
const { t } = useTranslation("page-roadmap") | ||
|
||
const releasesData = useMemo(() => getReleasesData(t), [t]) |
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.
Potentially out-of-scope for these changes, but our "data" file seems more like a hook... we could consider refactoring getReleasesData
to a hook, with the other hooks embedded
ie create app/[locale]/roadmap/hooks/useReleases.tsx
, which could contain everything in src/data/roadmap/releases.tsx
, as well as the other client-side hooks here, returning everything we need:
const {
releasesData,
startIndex,
setApi1,
setApi2,
getStatus,
getDisplayDate,
currentIndex,
t
} = useReleases()
Fixes external link handling; fixes broken glossary tooltip using standard Link instead
Noticed our use of Also noticed the glossary tooltip links were not properly render the term/definition strings in the popup so I converted to a simple Link for now—can further debug that separately... Letting it build then will pull this in. |
Preview link
https://deploy-preview-16057--ethereumorg.netlify.app/en/roadmap