refactor(web): timezone select options display with i18n support#2497
refactor(web): timezone select options display with i18n support#2497bowenliang123 wants to merge 4 commits intoagentscope-ai:mainfrom
Conversation
|
Hi @bowenliang123, this is your 7th Pull Request. 🙌 Join Developer CommunityThanks so much for your contribution! We'd love to invite you to join the official CoPaw developer group! You can find the Discord and DingTalk group links under the "Developer Community" section on our docs page: We truly appreciate your enthusiasm—and look forward to your future contributions! 😊 We'll review your PR soon. |
There was a problem hiding this comment.
Code Review
This pull request replaces the static timezone list with a dynamic, localized implementation using the @vvo/tzdb library and a new useTimezoneOptions hook. This update enables translated timezone labels with UTC offsets in the agent configuration and cron job components. Feedback suggests improving the scalability of the locale mapping and refactoring the timezone mapping logic to avoid redundancy.
There was a problem hiding this comment.
Pull request overview
Refactors the console’s timezone <Select> options to be generated dynamically (with current offsets) and localized via the current i18n language, replacing the prior hardcoded label strings.
Changes:
- Replaced static timezone option labels with
@vvo/tzdb-backed option generation (getTimezoneOptions) and localized display names viaIntl.DateTimeFormat. - Introduced
useTimezoneOptions()hook and switched CronJobs + Agent Config selects to use it. - Updated exports/imports to remove
TIMEZONE_OPTIONSre-exports from CronJobs components.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| console/src/pages/Control/CronJobs/components/index.ts | Stops re-exporting TIMEZONE_OPTIONS from CronJobs components. |
| console/src/pages/Control/CronJobs/components/constants.ts | Removes passthrough export of timezone options; keeps form defaults only. |
| console/src/pages/Control/CronJobs/components/JobDrawer.tsx | Switches timezone <Select> options to useTimezoneOptions(). |
| console/src/pages/Agent/Config/components/ReactAgentCard.tsx | Switches timezone <Select> options to useTimezoneOptions(). |
| console/src/hooks/useTimezoneOptions.ts | Adds hook to memoize timezone options by current i18n.language. |
| console/src/constants/timezone.ts | Implements tzdb-backed, localized timezone option generation. |
| console/package.json | Adds @vvo/tzdb dependency. |
| console/package-lock.json | Locks @vvo/tzdb dependency. |
Files not reviewed (1)
- console/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you for your contribution! Or refer to https://github.com/agentscope-ai/CoPaw/blob/main/CONTRIBUTING.md#4-code-and-quality |
|
Fixed the lint issues. Sorry to forgot to reformat the code again. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- console/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .toLowerCase() | ||
| .includes(input.toLowerCase()) | ||
| } | ||
| options={TIMEZONE_OPTIONS} | ||
| options={useTimezoneOptions()} | ||
| /> |
There was a problem hiding this comment.
useTimezoneOptions() is a React hook; calling it directly inside JSX props (in the options={...} expression) will trip react-hooks/rules-of-hooks and can break hook ordering if this render path ever becomes conditional. Call the hook once at the top of the component (e.g., assign to a timezoneOptions const) and pass that variable to Select instead.
| .toLowerCase() | ||
| .includes(input.toLowerCase()) | ||
| } | ||
| options={TIMEZONE_OPTIONS} | ||
| options={useTimezoneOptions()} | ||
| onChange={onTimezoneChange} |
There was a problem hiding this comment.
useTimezoneOptions() is a React hook; calling it inline in options={useTimezoneOptions()} violates react-hooks/rules-of-hooks (this repo enables the recommended react-hooks rules) and may fail linting. Invoke the hook once near the top of ReactAgentCard and pass the resulting array to the Select.
|
|
||
| export function useTimezoneOptions(): TimezoneOption[] { | ||
| const { i18n } = useTranslation(); | ||
| return useMemo(() => getTimezoneOptions(i18n.language), [i18n.language]); |
There was a problem hiding this comment.
i18n.language can include region subtags (e.g. en-US). In this repo App.tsx normalizes language via lng.split("-")[0] when deriving locale. Consider normalizing here too (and/or using i18n.resolvedLanguage) before passing into getTimezoneOptions, otherwise the locale map in getLocalizedName will fall back to English unexpectedly.
| return useMemo(() => getTimezoneOptions(i18n.language), [i18n.language]); | |
| const language = i18n.resolvedLanguage ?? i18n.language; | |
| return useMemo( | |
| () => { | |
| const locale = (language ?? "en").split("-")[0]; | |
| return getTimezoneOptions(locale); | |
| }, | |
| [language], | |
| ); |
| .sort((a, b) => a.currentTimeOffsetInMinutes - b.currentTimeOffsetInMinutes) | ||
| .map((tz) => { | ||
| const value = tz.name === "Etc/UTC" ? "UTC" : tz.name; | ||
| return { | ||
| value, | ||
| label: `${getLocalizedName(tz.name, lang)} (${ | ||
| tz.currentTimeFormat.split(" ")[0] | ||
| }, ${value})`, |
There was a problem hiding this comment.
The offset portion of the label is derived from tz.currentTimeFormat.split(" ")[0], which is brittle and depends on the library’s string format (and may include a UTC prefix, differing from the intended +08:00 display). Prefer formatting tz.currentTimeOffsetInMinutes into a ±HH:MM string directly so the UI output is stable and matches the desired format.
Description
[Describe what this PR does and why]
Replace static timezone list with
@vvo/tzdblibrary for accurate offset data, to avoid hardcoded display text for timezonesAdd i18n support - timezone names now display in current page language (zh/en/ru/ja)
New display format:
Localized Name (+08:00, Asia/Shanghai)instead ofAsia/Shanghai (UTC+8)Before:
Related Issue:* Fixes #(issue_number) or Relates to #(issue_number)
Security Considerations: [If applicable, e.g. channel auth, env/config handling]
Type of Change
Component(s) Affected
Checklist
pre-commit run --all-fileslocally and it passespytestor as relevant) and they passTesting
[How to test these changes]
Local Verification Evidence
Additional Notes
[Optional: any other context]