Conversation
wisdomqin
left a comment
There was a problem hiding this comment.
Thanks for this PR, @liaozip! The work is solid — particularly the emoji cleanup and replacing hardcoded strings with t() calls across components. A few notes before we can merge:
Conflicts — This PR is currently dirty (conflicts with main). We've had significant changes to AgentDetail.tsx, en.json, and zh.json since your branch was created. A rebase is required.
Scope concerns — I'd like to separate a few items:
-
skillNames/skillDescriptionsin i18n — Preset skill content (Designer, Market Researcher descriptions, boundaries, etc.) should live in dedicated skill files, not in JSON translation files. Mixing business content with UI strings makes both harder to maintain. Please remove this section and we can handle it separately. -
Email template defaults in
AdminCompanies.tsx— Embedding full email body text as i18n keys is a pattern we want to avoid. The backend already providesd.defaultsfor this. The simpler fix is justsetEmailTemplates({ ...d.defaults, ...d.templates })— let's keep the logic there. -
UserManagementPlatform Admin badge — The\n+whiteSpace: pre-lineapproach for wrapping role labels feels fragile. Leave the label as a single line for now.
What we definitely want:
- Emoji removal from i18n ✓
- Hardcoded →
t()inAgentDetail.tsx,ChannelConfig.tsx,FileBrowser.tsx✓ CopyMessageButtonmissinguseTranslation()fix ✓UserManagementperiodLabeli18n refactor ✓.gitignore.trae/addition ✓
Suggested action: Rebase on main, drop the skillNames/skillDescriptions block and the email template defaults, then push. That should reduce the conflict surface considerably and let us merge quickly.
- Remove skillNames/skillDescriptions from i18n (should be in skill files) - Simplify email template defaults in AdminCompanies.tsx (use backend defaults) - Fix UserManagement platform admin badge (remove fragile \n + pre-line approach)
|
I’ve addressed all the feedback. Here are the changes made:
|
|
Thanks for all the work here, @liaozip! We can see you've already addressed the previous review feedback — the emoji cleanup, the skillNames/skillDescriptions removal, and the badge fixes all look good. That said, since the changes in this PR touch quite a few different areas (i18n refactoring, hardcoded string replacement, UI tweaks, tool display names, etc.), the diff is a bit difficult to review thoroughly as a whole. We would like to close this PR for now and take a different approach. What we suggest instead: Please describe each individual need or improvement as a separate GitHub Issue. We will prioritize and implement them one by one in upcoming versions. This way nothing gets lost, and each change can be reviewed cleanly. We really appreciate your contribution and engagement with the project — your attention to the i18n details is exactly the kind of thing that improves the platform for everyone. Looking forward to working through these with you! |
已完成的工作:
修改示例: