-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: home language support without stickies #6443
base: preview
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces comprehensive internationalization (i18n) support across multiple components in the Plane application. The changes primarily focus on adding translation capabilities to various frontend components, updating translation files for multiple languages (English, Spanish, French, Japanese, and Chinese), and modifying type definitions to support localization. The modifications enable dynamic text rendering based on the user's language preferences, enhancing the application's global accessibility. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI
participant TranslationService
participant LocaleFiles
User->>UI: Change Language
UI->>TranslationService: Request Translations
TranslationService->>LocaleFiles: Fetch Translation Keys
LocaleFiles-->>TranslationService: Return Translated Strings
TranslationService-->>UI: Provide Localized Text
UI->>User: Display Localized Interface
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 7
🧹 Nitpick comments (13)
web/core/components/home/widgets/empty-states/links.tsx (1)
2-2
: LGTM! Consider adding type safety for translations.The i18n implementation is clean and follows best practices. Consider using typed translation keys to prevent runtime errors from typos or missing translations.
// Add type safety for translation keys import { TFunction } from "@plane/i18n";Also applies to: 4-14
web/core/components/home/widgets/empty-states/stickies.tsx (1)
2-2
: Maintain consistent translation key hierarchy.The translation key
stickies.empty
doesn't follow the same hierarchical pattern as other home components (e.g.,home.quick_links.empty
). Consider updating for consistency:- <div className="text-custom-text-400 text-sm text-center my-auto">{t("stickies.empty")}</div> + <div className="text-custom-text-400 text-sm text-center my-auto">{t("home.stickies.empty")}</div>Also applies to: 6-6, 11-11
web/core/components/home/widgets/manage/index.tsx (1)
Line range hint
3-3
: Consider modern React patterns.The
FC
type is generally discouraged in modern React as it doesn't provide significant benefits and can mask type issues. Consider:-export const ManageWidgetsModal: FC<TProps> = observer((props) => { +export const ManageWidgetsModal = observer((props: TProps) => {Also applies to: 16-16
web/core/components/home/widgets/empty-states/recents.tsx (1)
2-2
: Consider adding error handling for invalid type values.While the translation implementation is correct, the switch case doesn't handle invalid type values explicitly. Consider adding error handling or type validation.
const getDisplayContent = () => { + if (!["project", "page", "issue"].includes(type)) { + console.warn(`Invalid type "${type}" provided to RecentsEmptyState`); + return { + icon: <History size={30} className="text-custom-text-400/40" />, + text: t("home.recents.empty.default"), + }; + } switch (type) { case "project": return {Also applies to: 6-6, 12-12, 17-17, 22-22, 27-27
web/app/[workspaceSlug]/(projects)/header.tsx (1)
10-10
: Consider accessibility improvement for GitHub star link.While the translation implementation is correct, the hidden text might affect screen readers. Consider using
sr-only
class instead ofhidden
for better accessibility.- <span className="hidden text-xs font-medium sm:hidden md:block">{t("home.star_us_on_github")}</span> + <span className="sr-only">{t("home.star_us_on_github")}</span> + <span className="hidden text-xs font-medium sm:hidden md:block">{t("home.star_us_on_github")}</span>Also applies to: 23-23, 33-35, 58-58
web/core/components/home/user-greetings.tsx (1)
Line range hint
43-45
: Extract time-based logic into a separate function.The greeting and emoji selection logic could be extracted into a utility function for better maintainability and reusability.
Consider creating a utility function:
const getTimeBasedGreeting = (hour: number) => { const greeting = hour < 12 ? "morning" : hour < 18 ? "afternoon" : "evening"; const emoji = greeting === "morning" ? "🌤️" : greeting === "afternoon" ? "🌥️" : "🌙️"; return { greeting, emoji }; };Also applies to: 53-57
web/core/components/home/widgets/links/root.tsx (2)
39-42
: Remove unnecessary whitespace in JSX.The explicit whitespace character and empty lines make the code less maintainable.
Apply this diff:
- <div className="text-base font-semibold text-custom-text-350"> - {" "} - {t("home.quick_links.title", { count: 2 })} - </div> + <div className="text-base font-semibold text-custom-text-350"> + {t("home.quick_links.title", { count: 2 })} + </div>
43-50
: Enhance button accessibility.The button lacks proper ARIA attributes for better accessibility.
Apply this diff:
<button onClick={() => { toggleLinkModal(true); }} + aria-label={t("home.quick_links.add")} className="flex gap-1 text-sm font-medium text-custom-primary-100 my-auto" > <Plus className="size-4 my-auto" /> <span>{t("home.quick_links.add")}</span>
web/core/components/home/widgets/manage/widget-list.tsx (2)
Line range hint
17-46
: Improve error handling in handleDrop function.The current error handling doesn't provide specific error information to users. Consider capturing and translating specific error cases.
Apply this diff:
try { reorderWidget(workspaceSlug, sourceData.id, droppedId, instruction); setToast({ type: TOAST_TYPE.SUCCESS, title: t("toast.success"), message: t("home.widget.reordered_successfully"), }); - } catch { + } catch (error) { setToast({ type: TOAST_TYPE.ERROR, title: t("toast.error"), - message: t("home.widget.reordering_failed"), + message: t("home.widget.reordering_failed_with_reason", { + reason: error instanceof Error ? error.message : t("common.unknown_error") + }), }); }
Line range hint
17-31
: Consider extracting drop target validation logic.The drop target validation logic in handleDrop is complex and could be extracted for better maintainability.
Consider creating a separate function:
const validateDropTarget = ( dropTargets: DropTargetRecord[], ): DropTargetRecord | null => { if (!dropTargets?.length) return null; return dropTargets.length > 1 ? dropTargets.find((target) => target?.data?.isChild) : dropTargets[0]; };web/core/components/home/widgets/links/link-detail.tsx (1)
44-45
: Consider aligning translation key structure with other files.The translation keys here are flat (e.g., "edit", "link_copied") while other files use a hierarchical structure. Consider using a consistent pattern:
-t("edit") +t("quick_links.actions.edit") -t("link_copied") +t("quick_links.toasts.link_copied.title")Also applies to: 54-54, 60-60, 66-66, 72-72
web/core/components/home/widgets/recents/index.tsx (1)
8-8
: LGTM! Consider enhancing type safety for i18n keys.The i18n implementation for filters looks good. Consider creating a type for the i18n keys to prevent typos and enable better IDE support.
type THomeRecentI18nKeys = | "home.recents.filters.all" | "home.recents.filters.issues" | "home.recents.filters.pages" | "home.recents.filters.projects"; const filters: { name: TRecentActivityFilterKeys; icon?: React.ReactNode; i18n_key: THomeRecentI18nKeys }[] = [ // ... rest of the array ];Also applies to: 25-29
web/core/components/home/widgets/empty-states/root.tsx (1)
31-32
: LGTM! Consider extracting translation keys to constants.The translation keys are consistently structured but could benefit from being centralized in a constants file to avoid duplication and enable easier maintenance.
// constants/i18n-keys.ts export const HOME_EMPTY_KEYS = { CREATE_PROJECT: { TITLE: "home.empty.create_project.title", DESCRIPTION: "home.empty.create_project.description", CTA: "home.empty.create_project.cta" }, // ... other sections } as const;Also applies to: 35-35, 47-48, 51-51, 57-58, 61-61, 67-68, 88-88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
packages/i18n/src/locales/en/translations.json
(1 hunks)packages/i18n/src/locales/es/translations.json
(1 hunks)packages/i18n/src/locales/fr/translations.json
(1 hunks)packages/i18n/src/locales/ja/translations.json
(1 hunks)packages/i18n/src/locales/zh-CN/translations.json
(1 hunks)packages/types/src/home.d.ts
(1 hunks)web/app/[workspaceSlug]/(projects)/header.tsx
(4 hunks)web/app/[workspaceSlug]/(projects)/page.tsx
(2 hunks)web/core/components/core/content-overflow-HOC.tsx
(3 hunks)web/core/components/home/home-dashboard-widgets.tsx
(1 hunks)web/core/components/home/index.ts
(0 hunks)web/core/components/home/project-empty-state.tsx
(0 hunks)web/core/components/home/user-greetings.tsx
(4 hunks)web/core/components/home/widgets/empty-states/links.tsx
(1 hunks)web/core/components/home/widgets/empty-states/recents.tsx
(1 hunks)web/core/components/home/widgets/empty-states/root.tsx
(6 hunks)web/core/components/home/widgets/empty-states/stickies.tsx
(1 hunks)web/core/components/home/widgets/links/action.tsx
(2 hunks)web/core/components/home/widgets/links/create-update-link-modal.tsx
(6 hunks)web/core/components/home/widgets/links/link-detail.tsx
(4 hunks)web/core/components/home/widgets/links/root.tsx
(3 hunks)web/core/components/home/widgets/links/use-links.tsx
(3 hunks)web/core/components/home/widgets/manage/index.tsx
(2 hunks)web/core/components/home/widgets/manage/widget-item.tsx
(3 hunks)web/core/components/home/widgets/manage/widget-list.tsx
(2 hunks)web/core/components/home/widgets/recents/filters.tsx
(3 hunks)web/core/components/home/widgets/recents/index.tsx
(5 hunks)
💤 Files with no reviewable changes (2)
- web/core/components/home/index.ts
- web/core/components/home/project-empty-state.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (14)
web/core/components/home/widgets/links/action.tsx (1)
2-2
: LGTM! Clean i18n implementation.The translation implementation follows best practices and maintains consistent key hierarchy.
Also applies to: 9-9, 19-19
web/core/components/home/widgets/manage/index.tsx (1)
7-7
: LGTM! Clean i18n implementation.The translation implementation follows best practices and maintains consistent key hierarchy.
Also applies to: 20-20, 25-25
web/app/[workspaceSlug]/(projects)/page.tsx (1)
5-5
: LGTM! Clean i18n implementation.The translation implementation is correct and consistent with other components.
Also applies to: 15-15, 17-17
packages/types/src/home.d.ts (1)
4-4
: Verify all usages of TRecentActivityFilterKeys.The change from "all item" to "all" is a breaking change. Let's verify all components using this type have been updated.
✅ Verification successful
No issues found with TRecentActivityFilterKeys usage
The codebase is already using the correct values ("all", "issue", "page", "project") in all implementations. No updates needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining usages of "all item" that might need updating rg -l "all item" --type ts --type tsx # Search for components using TRecentActivityFilterKeys ast-grep --pattern 'TRecentActivityFilterKeys'Length of output: 377
Script:
#!/bin/bash # Check implementation in both files echo "=== index.tsx ===" rg "TRecentActivityFilterKeys" -A 5 -B 5 web/core/components/home/widgets/recents/index.tsx echo -e "\n=== filters.tsx ===" rg "TRecentActivityFilterKeys" -A 5 -B 5 web/core/components/home/widgets/recents/filters.tsxLength of output: 2509
web/core/components/home/widgets/recents/filters.tsx (1)
15-15
: Consider backward compatibility for type changes.Adding the required
i18n_key
field to thefilters
type might break existing code. Consider making it optional withi18n_key?: string
or providing a migration guide.Run this script to check for potential breaking changes:
web/core/components/home/home-dashboard-widgets.tsx (1)
28-28
: LGTM! Consistent i18n key structure.The translation keys follow a clear hierarchical pattern (
home.<widget_name>.title
), making the structure maintainable and easy to understand.Also applies to: 33-33, 38-38, 43-43, 48-48
web/core/components/home/widgets/links/use-links.tsx (2)
57-59
: LGTM! Consistent toast message structure.The translation keys follow a clear pattern (
quick_links.toasts.<action>.<type>
), making maintenance and updates straightforward.Also applies to: 64-66, 76-78, 82-84
37-39
: Verify error message fallback behavior.While the i18n implementation looks good, the error message fallback combines server error with translation key:
error?.data?.error ?? "quick_links.toasts.not_created.message"This might lead to inconsistent error display if the server returns an error message in a different language.
Consider standardizing error handling to always use translated messages:
-error?.data?.error ?? "quick_links.toasts.not_created.message" +t("quick_links.toasts.not_created.message")Also applies to: 45-47
web/core/components/core/content-overflow-HOC.tsx (1)
3-3
: LGTM! Appropriate use of simple translation keys.Since this is a generic, reusable component, using simple translation keys ("show_less", "show_all") is appropriate. The implementation is clean and straightforward.
Also applies to: 35-37, 149-149
web/core/components/home/widgets/recents/index.tsx (2)
39-39
: LGTM! Clean implementation of title translations.The translation hook is properly initialized and the title translation is implemented correctly.
Also applies to: 75-75
92-92
: LGTM! Consistent translation key usage.Good practice: Using the same translation key consistently for the title across both empty and non-empty states.
web/core/components/home/widgets/links/create-update-link-modal.tsx (1)
72-72
: Verify translation completeness for form validation messages.While the implementation looks good overall, ensure that all possible form validation messages are included in the translation files. Also, consider using a more specific translation key for generic actions like "Cancel", "Update", "Adding", etc.
Also applies to: 77-78, 84-84, 94-94, 99-99, 103-104, 117-117, 127-127, 130-131
✅ Verification successful
All translation keys are properly defined, but consider namespacing generic actions
All form validation messages and UI texts are properly included in the translation files. However, for better maintainability, consider using more specific translation keys for generic actions (e.g.,
link.modal.actions.update
instead ofUpdate
).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if all form-related translation keys exist in the English locale rg -t json '"link\.modal\.' packages/i18n/src/locales/en/Length of output: 59
Script:
#!/bin/bash # Find all translation keys used in the file ast-grep --pattern 't($STR)' web/core/components/home/widgets/links/create-update-link-modal.tsx # Search for translation files and their content echo "=== Translation files ===" fd . packages/i18n/src/locales/en -e json -x cat {} # Search for common action translations echo "=== Common actions ===" rg -t json '"(Update|Add|Cancel)"' packages/i18n/src/locales/en/Length of output: 22826
web/core/components/home/widgets/manage/widget-item.tsx (1)
133-133
: Verify pluralization handling in translations.The code passes a count parameter to the translation function, but it's hardcoded to 1. This might limit proper pluralization support if needed in the future.
packages/i18n/src/locales/ja/translations.json (1)
369-369
: Verify pluralization format for Japanese.The pluralization format might not be appropriate for Japanese language:
"title": "クイック{count, plural, one{#リンク} other{#リンク}}"Please confirm if this pluralization format is necessary for Japanese, as the language typically doesn't distinguish between singular and plural forms in the same way as English.
@@ -27,18 +29,19 @@ export const FiltersDropdown: FC<TFiltersDropdown> = observer((props) => { | |||
}} | |||
> | |||
{filter.icon && <div>{filter.icon}</div>} | |||
<div className="truncate font-medium text-xs capitalize">{`${filter.name}s`}</div> | |||
<div className="truncate font-medium text-xs capitalize">{t(`${filter.i18n_key}`)}</div> |
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.
🛠️ Refactor suggestion
Avoid string concatenation in translation keys.
Using string concatenation with t() function (t(\
${filter.i18n_key}`)and
t(`${title}`)`) is not recommended as it can lead to missing translations. Use plain strings instead.
Apply this diff:
- <div className="truncate font-medium text-xs capitalize">{t(`${filter.i18n_key}`)}</div>
+ <div className="truncate font-medium text-xs capitalize">{t(filter.i18n_key)}</div>
- <span className="font-medium text-sm my-auto"> {t(`${title}`)}</span>
+ <span className="font-medium text-sm my-auto"> {t(title || "")}</span>
Also applies to: 44-44
{t("good")} {t(greeting)}, {user?.first_name} {user?.last_name} | ||
</h3> |
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.
🛠️ Refactor suggestion
Improve greeting translation structure.
The current implementation splits the greeting into multiple t() calls. Consider using a single translation key with interpolation for better maintainability and translation accuracy.
Apply this diff:
- {t("good")} {t(greeting)}, {user?.first_name} {user?.last_name}
+ {t("greetings.full", {
+ greeting,
+ firstName: user?.first_name || "",
+ lastName: user?.last_name || ""
+ })}
Committable suggestion skipped: line range outside the PR's diff.
"inbox": "收件箱", | ||
"show_all": "显示全部", | ||
"show_less": "显示较少" |
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.
Missing translations for new keys.
While "show_all" and "show_less" are added, several new keys present in other language files are missing:
- Home section structure
- Toast messages
- Link modal texts
- Quick links section
Please add translations for all missing keys to maintain consistency across languages. I can help generate a list of missing translations if needed.
"description": "サブテキストは2行になり、ここに配置されます。", | ||
"cta": "今すぐ招待" |
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.
Replace placeholder text with actual descriptions.
Several sections contain placeholder text that needs to be replaced with proper descriptions:
- Invite team description
- Configure workspace description
- Personalize account description
- "description": "サブテキストは2行になり、ここに配置されます。",
+ "description": "チームメンバーを招待して、共同作業を始めましょう。",
Also applies to: 357-358, 362-363
"title": "Invite your team", | ||
"description": "The sub text will be of two lines and that will be placed.", |
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.
Replace placeholder descriptions with meaningful content.
Several sections contain placeholder text that should be replaced with proper descriptions:
- Invite team description
- Configure workspace description
- Personalize account description
- "description": "The sub text will be of two lines and that will be placed.",
+ "description": "Invite your team members to collaborate on projects and tasks.",
Also applies to: 357-358, 362-363
"description": "El subtexto será de dos líneas y se colocará aquí.", | ||
"cta": "Invitar ahora" |
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.
Replace placeholder text with actual Spanish descriptions.
Several sections contain placeholder text that needs to be replaced with proper Spanish descriptions:
- Invite team description
- Configure workspace description
- Personalize account description
- "description": "El subtexto será de dos líneas y se colocará aquí.",
+ "description": "Invita a los miembros de tu equipo para comenzar a colaborar.",
Also applies to: 357-358, 362-363
"description": "Le sous-texte sera sur deux lignes et sera placé ici.", | ||
"cta": "Inviter maintenant" |
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.
Replace placeholder text with actual French descriptions.
Several sections contain placeholder text that needs to be replaced with proper French descriptions:
- Invite team description
- Configure workspace description
- Personalize account description
- "description": "Le sous-texte sera sur deux lignes et sera placé ici.",
+ "description": "Invitez les membres de votre équipe pour commencer à collaborer.",
Also applies to: 357-358, 362-363
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
Release Notes
Internationalization
Home Dashboard
User Experience
Minor Improvements