feat: implement internationalization support with next-intl#270
feat: implement internationalization support with next-intl#270shiverse94 wants to merge 2 commits into
Conversation
- Added support for English and Simplified Chinese locales. - Introduced message files for various components and pages. - Implemented locale-prefixed routing and validation script for message key parity. - Updated navigation and layout components to utilize internationalization features. Signed-off-by: Shive <shiverse94@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @shiverse94! It looks like this is your first PR to volcano-sh/dashboard 🎉 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the Volcano dashboard to a Next.js monorepo using TurboRepo, Shadcn UI, and tRPC, while introducing internationalization support for English and Simplified Chinese. The new architecture covers management pages for Jobs, Queues, Pods, and PodGroups. Review feedback primarily highlighted several instances of hardcoded user-facing strings in pagination components, form labels, tooltips, and chart legends that need to be integrated into the i18n system. Furthermore, the reviewer advised replacing brittle setTimeout patterns with tRPC query invalidation to ensure reliable UI updates following data mutations.
| onClick={() => onPageChange(page + 1)} | ||
| disabled={disabled || page >= safeTotalPages || total === 0} | ||
| > | ||
| Next |
There was a problem hiding this comment.
The 'Next' button label is hardcoded. It should be localized using the common.actions.next key.
| disabled={disabled} | ||
| /> | ||
| <Label htmlFor="reclaimable" className="text-sm font-normal cursor-pointer"> | ||
| Reclaimable |
There was a problem hiding this comment.
Hardcoded label 'Reclaimable' should be replaced with a translation key.
| <Label htmlFor="queue-name"> | ||
| Queue Name {!nameReadOnly && <span className="text-destructive">*</span>} | ||
| </Label> |
There was a problem hiding this comment.
Form labels should be localized. Use the useTranslations hook to fetch the label from the message files.
| return ( | ||
| <div className="flex items-center justify-between space-x-2 py-4"> | ||
| <div className="flex-1 text-sm text-muted-foreground"> | ||
| Showing {startItem} to {endItem} of {total} results |
There was a problem hiding this comment.
This user-facing string is hardcoded. Since this PR implements internationalization, this should be moved to the message files (e.g., common.json) and accessed via useTranslations.
| </div> | ||
| <div className="flex items-center space-x-2"> | ||
| <div className="flex items-center space-x-2"> | ||
| <span className="text-sm text-muted-foreground">Show:</span> |
There was a problem hiding this comment.
Hardcoded UI label found. Please use the translation hook to maintain consistency with the rest of the application's i18n implementation.
| setTimeout(async () => { | ||
| await handleRefresh() | ||
| }, 2000) |
There was a problem hiding this comment.
Avoid using setTimeout for data synchronization. Use tRPC's query invalidation to ensure the UI stays in sync with the server state after a successful deletion.
await utils.queueRouter.getQueues.invalidate();
| setTimeout(async () => { | ||
| await handleRefresh() | ||
| }, 2000) |
There was a problem hiding this comment.
Using setTimeout to refresh data after a mutation is brittle and can lead to race conditions or stale UI if the network is slow. Since you are using tRPC, you should leverage the utils object to invalidate the relevant queries immediately upon success.
await utils.jobsRouter.getJobs.invalidate();
|
|
||
| const chartConfig = { | ||
| allocated: { | ||
| label: "Allocated", |
There was a problem hiding this comment.
Chart legend labels should be localized to support the Simplified Chinese locale added in this PR.
| setTimeout(async () => { | ||
| await handleRefresh() | ||
| }, 2000) |
There was a problem hiding this comment.
Query invalidation is preferred over setTimeout for refreshing lists after mutations. This ensures the most up-to-date data is fetched as soon as the server processes the request.
await utils.podRouter.getPods.invalidate();
| color: "hsl(var(--chart-1))", | ||
| }, | ||
| capacity: { | ||
| label: "Capability", |
There was a problem hiding this comment.
Chart legend labels should be localized to support the Simplified Chinese locale added in this PR.
…hive-new-chinese-trans
|
Adding label DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
2 participants
feat: implement internationalization support with next-intl
I created this branch from Deep’s PR for the new dashboard. Once his PR is merged, this branch can also be merged since it was created from his branch. Looking forward to contributing to and collaborating on the dashboard project.
@JesseStutler @de6p
Changes -> 217b0fc