Skip to content

feat(i18n): localize remaining hardcoded UI surfaces#9082

Open
xuhongbo wants to merge 3 commits into
makeplane:previewfrom
xuhongbo:i18n/localize-hardcoded-ui-v2
Open

feat(i18n): localize remaining hardcoded UI surfaces#9082
xuhongbo wants to merge 3 commits into
makeplane:previewfrom
xuhongbo:i18n/localize-hardcoded-ui-v2

Conversation

@xuhongbo
Copy link
Copy Markdown

Summary

Supersedes #9069 and rebases the i18n hardcoded-UI work on top of the current preview branch after #8898.

This version follows the new react-i18next namespace JSON structure from #8898 instead of adding a catch-all namespace. New strings are added to the relevant existing namespace files:

  • auth
  • common
  • editor
  • empty-state
  • inbox
  • page
  • project
  • work-item
  • workspace

Scope

  • Localizes remaining hardcoded UI strings across Web onboarding, inbox, workspace invitations, workspace views, page access, and project settings empty states.
  • Localizes Space public/auth/work-item surfaces, including public board errors, filters, issue peek details, reactions, comments, OAuth, login, and instance failure states.
  • Localizes editor link editing labels and descriptions.
  • Adds matching keys across all supported locales.

Verification

  • pnpm --filter=@plane/i18n run check:sync
  • pnpm fix:format
  • pnpm turbo run check:lint --filter=@plane/i18n --filter=web --filter=space --filter=@plane/editor --filter=@plane/propel
  • pnpm turbo run check:types --filter=@plane/i18n --filter=web --filter=space --filter=@plane/editor --filter=@plane/propel

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Important

Review skipped

Too many files!

This PR contains 254 files, which is 104 over the limit of 150.

To get a review, narrow the scope:
• coderabbit review --type committed # exclude uncommitted changes
• coderabbit review --dir # limit to a subdirectory
• coderabbit review --base # compare against a closer base

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d8828a56-14a2-46f2-9dcd-9b56862fb409

📥 Commits

Reviewing files that changed from the base of the PR and between 50a7b47 and ae52e18.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (254)
  • apps/space/app/error.tsx
  • apps/space/app/not-found.tsx
  • apps/space/components/account/auth-forms/auth-header.tsx
  • apps/space/components/account/auth-forms/auth-root.tsx
  • apps/space/components/account/auth-forms/email.tsx
  • apps/space/components/account/auth-forms/password.tsx
  • apps/space/components/account/auth-forms/unique-code.tsx
  • apps/space/components/account/terms-and-conditions.tsx
  • apps/space/components/account/user-logged-in.tsx
  • apps/space/components/common/powered-by.tsx
  • apps/space/components/instance/instance-failure-view.tsx
  • apps/space/components/issues/filters/labels.tsx
  • apps/space/components/issues/filters/root.tsx
  • apps/space/components/issues/filters/selection.tsx
  • apps/space/components/issues/filters/state.tsx
  • apps/space/components/issues/issue-layouts/error.tsx
  • apps/space/components/issues/issue-layouts/issue-layout-HOC.tsx
  • apps/space/components/issues/issue-layouts/kanban/default.tsx
  • apps/space/components/issues/issue-layouts/kanban/swimlanes.tsx
  • apps/space/components/issues/issue-layouts/list/default.tsx
  • apps/space/components/issues/issue-layouts/properties/all-properties.tsx
  • apps/space/components/issues/issue-layouts/properties/cycle.tsx
  • apps/space/components/issues/issue-layouts/properties/due-date.tsx
  • apps/space/components/issues/issue-layouts/properties/labels.tsx
  • apps/space/components/issues/issue-layouts/properties/member.tsx
  • apps/space/components/issues/issue-layouts/properties/modules.tsx
  • apps/space/components/issues/issue-layouts/properties/priority.tsx
  • apps/space/components/issues/issue-layouts/properties/state.tsx
  • apps/space/components/issues/issue-layouts/utils.tsx
  • apps/space/components/issues/navbar/user-avatar.tsx
  • apps/space/components/issues/peek-overview/comment/add-comment.tsx
  • apps/space/components/issues/peek-overview/comment/comment-detail-card.tsx
  • apps/space/components/issues/peek-overview/header.tsx
  • apps/space/components/issues/peek-overview/issue-activity.tsx
  • apps/space/components/issues/peek-overview/issue-properties.tsx
  • apps/space/components/issues/reactions/issue-vote-reactions.tsx
  • apps/space/components/ui/not-found.tsx
  • apps/space/hooks/oauth/core.tsx
  • apps/web/app/(all)/[workspaceSlug]/(projects)/profile/[userId]/activity/page.tsx
  • apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx
  • apps/web/app/(all)/[workspaceSlug]/(projects)/sidebar.tsx
  • apps/web/app/(all)/[workspaceSlug]/(projects)/stickies/page.tsx
  • apps/web/app/(all)/[workspaceSlug]/(projects)/workspace-views/page.tsx
  • apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/page.tsx
  • apps/web/app/(all)/workspace-invitations/page.tsx
  • apps/web/app/not-found.tsx
  • apps/web/ce/components/projects/settings/intake/header.tsx
  • apps/web/core/components/inbox/content/inbox-issue-header.tsx
  • apps/web/core/components/inbox/content/inbox-issue-mobile-header.tsx
  • apps/web/core/components/inbox/content/issue-properties.tsx
  • apps/web/core/components/inbox/content/issue-root.tsx
  • apps/web/core/components/inbox/inbox-filter/applied-filters/date.tsx
  • apps/web/core/components/inbox/inbox-filter/applied-filters/label.tsx
  • apps/web/core/components/inbox/inbox-filter/applied-filters/root.tsx
  • apps/web/core/components/inbox/inbox-filter/applied-filters/state.tsx
  • apps/web/core/components/inbox/inbox-filter/applied-filters/status.tsx
  • apps/web/core/components/inbox/inbox-filter/filters/date.tsx
  • apps/web/core/components/inbox/inbox-filter/filters/filter-selection.tsx
  • apps/web/core/components/inbox/inbox-filter/filters/labels.tsx
  • apps/web/core/components/inbox/inbox-filter/filters/members.tsx
  • apps/web/core/components/inbox/inbox-filter/filters/state.tsx
  • apps/web/core/components/inbox/inbox-filter/filters/status.tsx
  • apps/web/core/components/inbox/inbox-filter/root.tsx
  • apps/web/core/components/inbox/modals/create-modal/create-root.tsx
  • apps/web/core/components/inbox/modals/create-modal/issue-description.tsx
  • apps/web/core/components/inbox/modals/create-modal/issue-properties.tsx
  • apps/web/core/components/inbox/modals/select-duplicate.tsx
  • apps/web/core/components/inbox/sidebar/inbox-list-item.tsx
  • apps/web/core/components/inbox/sidebar/root.tsx
  • apps/web/core/components/onboarding/invitations.tsx
  • apps/web/core/components/onboarding/invite-members.tsx
  • apps/web/core/components/onboarding/steps/profile/root.tsx
  • apps/web/core/components/onboarding/steps/profile/set-password.tsx
  • apps/web/core/components/onboarding/steps/team/root.tsx
  • apps/web/core/components/onboarding/steps/usecase/root.tsx
  • apps/web/core/components/onboarding/steps/workspace/create.tsx
  • apps/web/core/components/onboarding/steps/workspace/join-invites.tsx
  • apps/web/core/components/stickies/modal/stickies.tsx
  • apps/web/core/layouts/auth-layout/workspace-wrapper.tsx
  • packages/editor/package.json
  • packages/editor/src/core/components/links/link-edit-view.tsx
  • packages/i18n/src/locales/cs/auth.json
  • packages/i18n/src/locales/cs/common.json
  • packages/i18n/src/locales/cs/editor.json
  • packages/i18n/src/locales/cs/empty-state.json
  • packages/i18n/src/locales/cs/inbox.json
  • packages/i18n/src/locales/cs/page.json
  • packages/i18n/src/locales/cs/project.json
  • packages/i18n/src/locales/cs/work-item.json
  • packages/i18n/src/locales/cs/workspace.json
  • packages/i18n/src/locales/de/auth.json
  • packages/i18n/src/locales/de/common.json
  • packages/i18n/src/locales/de/editor.json
  • packages/i18n/src/locales/de/empty-state.json
  • packages/i18n/src/locales/de/inbox.json
  • packages/i18n/src/locales/de/page.json
  • packages/i18n/src/locales/de/project.json
  • packages/i18n/src/locales/de/work-item.json
  • packages/i18n/src/locales/de/workspace.json
  • packages/i18n/src/locales/en/auth.json
  • packages/i18n/src/locales/en/common.json
  • packages/i18n/src/locales/en/editor.json
  • packages/i18n/src/locales/en/empty-state.json
  • packages/i18n/src/locales/en/inbox.json
  • packages/i18n/src/locales/en/page.json
  • packages/i18n/src/locales/en/project.json
  • packages/i18n/src/locales/en/work-item.json
  • packages/i18n/src/locales/en/workspace.json
  • packages/i18n/src/locales/es/auth.json
  • packages/i18n/src/locales/es/common.json
  • packages/i18n/src/locales/es/editor.json
  • packages/i18n/src/locales/es/empty-state.json
  • packages/i18n/src/locales/es/inbox.json
  • packages/i18n/src/locales/es/page.json
  • packages/i18n/src/locales/es/project.json
  • packages/i18n/src/locales/es/work-item.json
  • packages/i18n/src/locales/es/workspace.json
  • packages/i18n/src/locales/fr/auth.json
  • packages/i18n/src/locales/fr/common.json
  • packages/i18n/src/locales/fr/editor.json
  • packages/i18n/src/locales/fr/empty-state.json
  • packages/i18n/src/locales/fr/inbox.json
  • packages/i18n/src/locales/fr/page.json
  • packages/i18n/src/locales/fr/project.json
  • packages/i18n/src/locales/fr/work-item.json
  • packages/i18n/src/locales/fr/workspace.json
  • packages/i18n/src/locales/id/auth.json
  • packages/i18n/src/locales/id/common.json
  • packages/i18n/src/locales/id/editor.json
  • packages/i18n/src/locales/id/empty-state.json
  • packages/i18n/src/locales/id/inbox.json
  • packages/i18n/src/locales/id/page.json
  • packages/i18n/src/locales/id/project.json
  • packages/i18n/src/locales/id/work-item.json
  • packages/i18n/src/locales/id/workspace.json
  • packages/i18n/src/locales/it/auth.json
  • packages/i18n/src/locales/it/common.json
  • packages/i18n/src/locales/it/editor.json
  • packages/i18n/src/locales/it/empty-state.json
  • packages/i18n/src/locales/it/inbox.json
  • packages/i18n/src/locales/it/page.json
  • packages/i18n/src/locales/it/project.json
  • packages/i18n/src/locales/it/work-item.json
  • packages/i18n/src/locales/it/workspace.json
  • packages/i18n/src/locales/ja/auth.json
  • packages/i18n/src/locales/ja/common.json
  • packages/i18n/src/locales/ja/editor.json
  • packages/i18n/src/locales/ja/empty-state.json
  • packages/i18n/src/locales/ja/inbox.json
  • packages/i18n/src/locales/ja/page.json
  • packages/i18n/src/locales/ja/project.json
  • packages/i18n/src/locales/ja/work-item.json
  • packages/i18n/src/locales/ja/workspace.json
  • packages/i18n/src/locales/ko/auth.json
  • packages/i18n/src/locales/ko/common.json
  • packages/i18n/src/locales/ko/editor.json
  • packages/i18n/src/locales/ko/empty-state.json
  • packages/i18n/src/locales/ko/inbox.json
  • packages/i18n/src/locales/ko/page.json
  • packages/i18n/src/locales/ko/project.json
  • packages/i18n/src/locales/ko/work-item.json
  • packages/i18n/src/locales/ko/workspace.json
  • packages/i18n/src/locales/pl/auth.json
  • packages/i18n/src/locales/pl/common.json
  • packages/i18n/src/locales/pl/editor.json
  • packages/i18n/src/locales/pl/empty-state.json
  • packages/i18n/src/locales/pl/inbox.json
  • packages/i18n/src/locales/pl/page.json
  • packages/i18n/src/locales/pl/project.json
  • packages/i18n/src/locales/pl/work-item.json
  • packages/i18n/src/locales/pl/workspace.json
  • packages/i18n/src/locales/pt-BR/auth.json
  • packages/i18n/src/locales/pt-BR/common.json
  • packages/i18n/src/locales/pt-BR/editor.json
  • packages/i18n/src/locales/pt-BR/empty-state.json
  • packages/i18n/src/locales/pt-BR/inbox.json
  • packages/i18n/src/locales/pt-BR/page.json
  • packages/i18n/src/locales/pt-BR/project.json
  • packages/i18n/src/locales/pt-BR/work-item.json
  • packages/i18n/src/locales/pt-BR/workspace.json
  • packages/i18n/src/locales/ro/auth.json
  • packages/i18n/src/locales/ro/common.json
  • packages/i18n/src/locales/ro/editor.json
  • packages/i18n/src/locales/ro/empty-state.json
  • packages/i18n/src/locales/ro/inbox.json
  • packages/i18n/src/locales/ro/page.json
  • packages/i18n/src/locales/ro/project.json
  • packages/i18n/src/locales/ro/work-item.json
  • packages/i18n/src/locales/ro/workspace.json
  • packages/i18n/src/locales/ru/auth.json
  • packages/i18n/src/locales/ru/common.json
  • packages/i18n/src/locales/ru/editor.json
  • packages/i18n/src/locales/ru/empty-state.json
  • packages/i18n/src/locales/ru/inbox.json
  • packages/i18n/src/locales/ru/page.json
  • packages/i18n/src/locales/ru/project.json
  • packages/i18n/src/locales/ru/work-item.json
  • packages/i18n/src/locales/ru/workspace.json
  • packages/i18n/src/locales/sk/auth.json
  • packages/i18n/src/locales/sk/common.json
  • packages/i18n/src/locales/sk/editor.json
  • packages/i18n/src/locales/sk/empty-state.json
  • packages/i18n/src/locales/sk/inbox.json
  • packages/i18n/src/locales/sk/page.json
  • packages/i18n/src/locales/sk/project.json
  • packages/i18n/src/locales/sk/work-item.json
  • packages/i18n/src/locales/sk/workspace.json
  • packages/i18n/src/locales/tr-TR/auth.json
  • packages/i18n/src/locales/tr-TR/common.json
  • packages/i18n/src/locales/tr-TR/editor.json
  • packages/i18n/src/locales/tr-TR/empty-state.json
  • packages/i18n/src/locales/tr-TR/inbox.json
  • packages/i18n/src/locales/tr-TR/page.json
  • packages/i18n/src/locales/tr-TR/project.json
  • packages/i18n/src/locales/tr-TR/work-item.json
  • packages/i18n/src/locales/tr-TR/workspace.json
  • packages/i18n/src/locales/ua/auth.json
  • packages/i18n/src/locales/ua/common.json
  • packages/i18n/src/locales/ua/editor.json
  • packages/i18n/src/locales/ua/empty-state.json
  • packages/i18n/src/locales/ua/inbox.json
  • packages/i18n/src/locales/ua/page.json
  • packages/i18n/src/locales/ua/project.json
  • packages/i18n/src/locales/ua/work-item.json
  • packages/i18n/src/locales/ua/workspace.json
  • packages/i18n/src/locales/vi-VN/auth.json
  • packages/i18n/src/locales/vi-VN/common.json
  • packages/i18n/src/locales/vi-VN/editor.json
  • packages/i18n/src/locales/vi-VN/empty-state.json
  • packages/i18n/src/locales/vi-VN/inbox.json
  • packages/i18n/src/locales/vi-VN/page.json
  • packages/i18n/src/locales/vi-VN/project.json
  • packages/i18n/src/locales/vi-VN/work-item.json
  • packages/i18n/src/locales/vi-VN/workspace.json
  • packages/i18n/src/locales/zh-CN/auth.json
  • packages/i18n/src/locales/zh-CN/common.json
  • packages/i18n/src/locales/zh-CN/editor.json
  • packages/i18n/src/locales/zh-CN/empty-state.json
  • packages/i18n/src/locales/zh-CN/inbox.json
  • packages/i18n/src/locales/zh-CN/page.json
  • packages/i18n/src/locales/zh-CN/project.json
  • packages/i18n/src/locales/zh-CN/work-item.json
  • packages/i18n/src/locales/zh-CN/workspace.json
  • packages/i18n/src/locales/zh-TW/auth.json
  • packages/i18n/src/locales/zh-TW/common.json
  • packages/i18n/src/locales/zh-TW/editor.json
  • packages/i18n/src/locales/zh-TW/empty-state.json
  • packages/i18n/src/locales/zh-TW/inbox.json
  • packages/i18n/src/locales/zh-TW/page.json
  • packages/i18n/src/locales/zh-TW/project.json
  • packages/i18n/src/locales/zh-TW/work-item.json
  • packages/i18n/src/locales/zh-TW/workspace.json
  • packages/propel/src/banner/banner.tsx
  • packages/propel/src/spinners/circular-spinner.tsx

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Localize remaining hardcoded UI strings across web, space, editor, and shared surfaces using the existing react-i18next namespace JSON files introduced by makeplane#8898.

Add the new keys to the relevant auth, common, editor, empty-state, inbox, page, project, work-item, and workspace locale files across all supported locales.

Co-authored-by: Codex <noreply@openai.com>
@xuhongbo xuhongbo force-pushed the i18n/localize-hardcoded-ui-v2 branch from 0ca8de5 to 03c1a8d Compare May 15, 2026 03:35
@xuhongbo
Copy link
Copy Markdown
Author

@coderabbitai you can now review the entire change. (Skip .json files)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the list of changed files (non-JSON) in PR `#9082`
gh pr view 9082 --repo makeplane/plane --json files --jq '.files[].path' | grep -v '\.json$'

Length of output: 4774


🏁 Script executed:

#!/bin/bash
# Get the full diff for non-JSON files in PR `#9082`
gh pr diff 9082 --repo makeplane/plane | grep -v '^diff --git.*\.json' | python3 - <<'PYEOF'
import sys

content = sys.stdin.read()

# Split by diff headers
import re
chunks = re.split(r'(?=^diff --git )', content, flags=re.MULTILINE)

# Filter out JSON file diffs
non_json_chunks = [c for c in chunks if not re.match(r'diff --git a/.*\.json', c)]

print(''.join(non_json_chunks))
PYEOF

Length of output: 1


🏁 Script executed:

#!/bin/bash
# Get the diff differently
gh api repos/makeplane/plane/pulls/9082/files --paginate --jq '.[] | select(.filename | endswith(".json") | not) | "=== FILE: \(.filename) ===\n\(.patch // "no patch")"' 2>/dev/null | head -5000

Length of output: 203988


@xuhongbo — here is the full review of the non-JSON changes in this PR. Overall the approach is solid and the i18n coverage is thorough; a few items below are worth addressing.


🔍 Code Review

🐛 Issues / Potential Regressions

1. autoFocus removed from the link edit dialog's URL input (packages/editor/src/core/components/links/link-edit-view.tsx)

The autoFocus prop was dropped from InputView entirely. Unlike page-load auto-focus (which has SSR/testing concerns), this dialog is opened by user action, so auto-focusing the URL field is the correct UX. Without it, users must manually click/tab before typing.

-function InputView({ label, value, placeholder, onChange, autoFocus }: InputViewProps) {
+function InputView({ label, value, placeholder, onChange }: InputViewProps) {
   ...
-      autoFocus={autoFocus}

Suggestion: Keep autoFocus on the URL field, or use a useEffect(() => { ref.current?.focus(); }, []) pattern like the email form does in this same PR, so the first input receives focus when the dialog opens.


2. autoFocus removed from filter search inputs

Both apps/web/core/components/inbox/inbox-filter/filters/filter-selection.tsx and apps/space/components/issues/filters/selection.tsx had autoFocus (or autoFocus={!isMobile}) removed from their search inputs. For a dropdown that opens on user action, this means desktop users need an extra click before they can type. The email form in this PR uses useEffect instead — consider applying the same pattern here.


3. Duplicate PAST_DURATION_FILTER_I18N_KEYS map

The exact same constant is defined in two files:

  • apps/web/core/components/inbox/inbox-filter/filters/date.tsx
  • apps/web/core/components/inbox/inbox-filter/applied-filters/date.tsx

Consider extracting it to a shared location (e.g. a constants.ts in the inbox filter directory) to keep it DRY.


4. Hardcoded English defaults in Spinner and Banner components

// packages/propel/src/spinners/circular-spinner.tsx
export function Spinner({ label = "Loading..." }: ISpinner)

// packages/propel/src/banner/banner.tsx
dismissLabel = "Dismiss banner",

These propel-level components are locale-agnostic — the defaults will always render English regardless of the active locale. Since callers already hold a t() instance, it is better to make these props required (or document that callers must always pass the translated string) rather than silently falling back to English.


5. Fragile sentence-splitting for Terms & Conditions (apps/space/components/account/terms-and-conditions.tsx)

{isSignUp ? t("space_public.by_creating_account") : t("space_public.by_signing_in")},{" "}
{t("space_public.you_agree_to_our")}

Splitting a single grammatical sentence across three separate keys is fragile for languages that have different word order or punctuation rules. A better pattern is to use a single key with an interpolated variable and render the link separately, or use Trans from react-i18next which handles inline elements within translated strings.


✅ Noteworthy Improvements Bundled In

These are positive changes worth acknowledging:

  • divbutton conversions throughout inbox filters applied-filters, sidebar tab navigation, onboarding invitations — good accessibility fixes.
  • Logic fix in join-invites.tsx: invitationsRespond.length <= 0 && !invitation?.role changed to || !selectedInvitation?.role. The original && would allow submission with no invitations selected as long as a role existed; || is correct.
  • tabIndex={1}tabIndex={0} in peek-overview/header.tsxtabIndex=1 breaks natural tab order; 0 is correct.
  • isButtonDisabled logic simplifications in password.tsx, usecase/root.tsx, profile/root.tsx — removes the confusing double-negation pattern.
  • Complete useMemo/useCallback dependency arrays (e.g. issue-root.tsx adding archiveIssue, projectId, removeIssue, t, workspaceSlug).
  • role="dialog" + aria-label added to link edit view container.
  • return added before router.push in async .then() callbacks — avoids unhandled floating promises.
  • onClick removed from <img> in profile/root.tsx — images with click handlers are an accessibility anti-pattern; the labeled button below it is the correct interaction target.
  • Shadow-variable renames (labellabelInfo, membermemberInfo, emailemailId, pageIdtargetPageId) — improves readability.

💬 Minor Notes

  • t("space_auth.confirm_password") is used for both the <label> and the placeholder in password.tsx. A dedicated placeholder key (e.g. space_auth.confirm_password_placeholder) would be more descriptive.
  • The error_description_prefix / error_description_middle key names in apps/space/app/error.tsx are not very descriptive — something like error_description_before_email / error_description_before_forum would communicate intent more clearly.
  • return undefined in .then() handlers (e.g. peek-overview/header.tsx, issue-properties.tsx) satisfies TypeScript's void return type but looks unusual — a comment or type cast would make the intent clearer to future readers.

xuhongbo and others added 2 commits May 15, 2026 12:09
Restore initial focus for the localized link editor and filter panels without using the autoFocus attribute, keeping the keyboard-first interaction while satisfying the a11y lint rules.

Co-authored-by: Codex <noreply@openai.com>
Tighten a small batch of locale strings against the English source for command-style actions, work item terminology, and workspace invitation messaging.

Co-authored-by: Codex <noreply@openai.com>
@xuhongbo
Copy link
Copy Markdown
Author

@coderabbitai review the latest changes. Skip .json files.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get all changed non-JSON files in PR `#9082` with their patches
gh api repos/makeplane/plane/pulls/9082/files --paginate \
  --jq '.[] | select(.filename | endswith(".json") | not) | "=== FILE: \(.filename) ===\nSTATUS: \(.status)\n\(.patch // "no patch")\n"' 2>/dev/null

Length of output: 207639


@xuhongbo — here is the review of the latest non-JSON changes.


🔍 Code Review

✅ Previous Issues Addressed

All the major items flagged in the last review have been resolved:

  • autoFocususeEffect + ref in link-edit-view.tsx, filter-selection.tsx (both apps/web and apps/space), and email.tsx.
  • tabIndex={1}tabIndex={0} in peek-overview/header.tsx.
  • isButtonDisabled double-negation cleaned up in password.tsx, usecase/root.tsx, profile/root.tsx.
  • useMemo dependency array completed in issue-root.tsx.
  • role="dialog" + aria-label added to the link edit view.
  • return before router.push in async callbacks.
  • onClick removed from <img> in profile/root.tsx.
  • Shadow-variable renames (label → labelInfo, member → memberInfo, email → emailId, value → roleDetail, pageId → targetPageId).
  • divbutton conversions throughout inbox filters, onboarding invitations, sidebar tabs, workspace-wrapper sign-out button.
  • join-invites.tsx logic fix: && correctly changed to ||.

🐛 Remaining / New Issues

1. Duplicate PAST_DURATION_FILTER_I18N_KEYS map (still present)

The identical constant is still defined in both:

  • apps/web/core/components/inbox/inbox-filter/filters/date.tsx
  • apps/web/core/components/inbox/inbox-filter/applied-filters/date.tsx

This was flagged in the previous review and is still open. Consider extracting it to a shared constants.ts in the inbox filter directory.


2. Spinner and Banner English defaults (still present)

// packages/propel/src/spinners/circular-spinner.tsx
label = "Loading..."

// packages/propel/src/banner/banner.tsx
dismissLabel = "Dismiss banner",

Both props are now properly wired (good progress), but the hardcoded English defaults remain. Callers that don't pass a translated string will still silently display English text. Since propel is locale-agnostic, these defaults should ideally be documented as "callers are required to pass a translated string," or the props should be made required.


3. Fragile Terms & Conditions string splitting (still present)

// apps/space/components/account/terms-and-conditions.tsx
{isSignUp ? t("space_public.by_creating_account") : t("space_public.by_signing_in")},{" "}
{t("space_public.you_agree_to_our")}

This is still the same split-key pattern. Languages with different word order or punctuation will produce unnatural output. The <Trans> component from react-i18next is the idiomatic solution for sentences with embedded links.


4. localizedLabels object reconstructed every render (missing useMemo)

In three files, a new localizedLabels object is created inline on every render:

// apps/space/components/issues/issue-layouts/kanban/default.tsx
const groupList = getGroupByColumns(..., {
  allWorkItems: `${t("common.all")} ${t("work_items")}`,
  none: t("common.none"),
});

// apps/space/components/issues/issue-layouts/kanban/swimlanes.tsx
const localizedLabels = {
  allWorkItems: `${t("common.all")} ${t("work_items")}`,
  none: t("common.none"),
};

// apps/space/components/issues/issue-layouts/list/default.tsx
const groupList = getGroupByColumns(..., {
  allWorkItems: `${t("common.all")} ${t("work_items")}`,
  none: t("common.none"),
});

Since t is stable across renders (same locale), this is functionally correct but allocates a new object reference on every render. In swimlanes.tsx the object is extracted to localizedLabels but still not memoized. Wrapping it in useMemo (or useRef initialized once) would be more efficient, especially for kanban boards that re-render frequently.

const localizedLabels = useMemo(() => ({
  allWorkItems: `${t("common.all")} ${t("work_items")}`,
  none: t("common.none"),
}), [t]);

5. archive parameters named with underscore prefix but immediately used

In apps/web/core/components/inbox/content/issue-root.tsx:

archive: async (_workspaceSlug: string, _projectId: string, issueId: string) => {
  try {
    await archiveIssue(_workspaceSlug, _projectId, issueId);
  } catch (error) { ... }
},

The convention of _ prefix signals "unused" to both TypeScript and linters, but _workspaceSlug and _projectId are actively used in the function body. This will likely trigger linter warnings. Either remove the underscores, or capture the outer workspaceSlug/projectId from the closure (which is the simpler fix since those values are already in scope).


💬 Minor Notes

  • apps/space/app/error.tsx and instance-failure-view.tsx both move handleRetry to module scope — this is a valid pattern but means the function is shared across all instances/renders, which is fine for a simple window.location.reload().
  • apps/web/core/components/inbox/content/inbox-issue-header.tsx: handleInboxIssueDelete now calls deleteInboxIssue and then unconditionally calls router.push outside the .then() — this means the redirect fires even if the delete fails. The original .then() chaining guarded the redirect. Consider wrapping in try/catch.
  • t("space_auth.confirm_password") is used for both the <label> and placeholder in password.tsx — still uses the same key for both (noted in prior review as a minor item, still present).

@xuhongbo
Copy link
Copy Markdown
Author

@codex review the latest changes. Skip .json files.

@xuhongbo
Copy link
Copy Markdown
Author

@claude review the latest changes. Skip .json files.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ae52e18462

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +85 to +88
<button
type="button"
key={invitation.id}
className={`flex cursor-pointer items-center gap-2 rounded-lg border border-subtle px-3 py-2 hover:bg-surface-2`}
className={`flex w-full cursor-pointer items-center gap-2 rounded-lg border border-subtle px-3 py-2 text-left hover:bg-surface-2`}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep invitation row non-button when rendering checkbox

This row was switched to a <button> while it still contains <Checkbox />, and Checkbox renders an actual <input type="checkbox">. Nesting one interactive control inside another is invalid HTML and causes inconsistent keyboard/screen-reader behavior (the row and checkbox become competing controls), so invitation selection can become unreliable for assistive-tech users. Keep the row as a non-button container with explicit click/keyboard handlers, or render a non-interactive check indicator inside the button.

Useful? React with 👍 / 👎.

@xuhongbo
Copy link
Copy Markdown
Author

@coderabbitai I reviewed the remaining items.\n\n- : agreed the / names are misleading since they are used. I am treating that as a naming cleanup, not a behavioral issue for this PR.\n- : the current flow is and only then . With normal async semantics, the redirect does not run if the delete call rejects, so I am not treating this as a confirmed regression from the current code.\n- The duplicated date filter map, / English defaults, and the split Terms & Conditions sentence are valid follow-up cleanup ideas, but they are outside the scope of this i18n PR.\n\nI am keeping this PR focused on localized strings and the minimum code changes required to preserve upstream behavior.

@xuhongbo
Copy link
Copy Markdown
Author

@coderabbitai I reviewed the remaining items.

  • issue-root.tsx: agreed the _workspaceSlug / _projectId names are misleading since they are used. I am treating that as a naming cleanup, not a behavioral issue for this PR.
  • inbox-issue-header.tsx: the current flow is await deleteInboxIssue(...) and only then router.push(...). With normal async semantics, the redirect does not run if the delete call rejects, so I am not treating this as a confirmed regression from the current code.
  • The duplicated date filter map, Spinner / Banner English defaults, and the split Terms & Conditions sentence are valid follow-up cleanup ideas, but they are outside the scope of this i18n PR.

I am keeping this PR focused on localized strings and the minimum code changes required to preserve upstream behavior.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Tip

For best results, initiate chat on the files or code changes.

@xuhongbo — thanks for the thorough walkthrough. All of the reasoning is sound:

  • The _-prefixed-but-used parameters are indeed a style/naming cleanup with no behavioral impact; happy to treat them that way.
  • On handleInboxIssueDelete: you're right that a rejected await will throw before the subsequent router.push line is reached, so the guard is still in place — I withdraw that concern.
  • The three deferred items (duplicate PAST_DURATION_FILTER_I18N_KEYS, Spinner/Banner English-only defaults, <Trans>-based Terms & Conditions) are reasonable follow-up candidates.

Would you like me to open follow-up GitHub issues for the three deferred items so they don't get lost? I can assign them to you and backlink this PR.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Tip

For best results, initiate chat on the files or code changes.

@xuhongbo — understood on all three points, thanks for the clear explanations:

  • issue-root.tsx — naming-only cleanup, agreed.
  • inbox-issue-header.tsxawait before router.push provides the guard; concern withdrawn.
  • Deferred items — all three (duplicate PAST_DURATION_FILTER_I18N_KEYS, Spinner/Banner English defaults, <Trans>-based Terms & Conditions) are reasonable follow-ups but out of scope here.

Would you like me to open GitHub issues for the three deferred items so they don't get lost between PRs? I can create them now and backlink this PR.

@xuhongbo
Copy link
Copy Markdown
Author

@coderabbitai Thanks. Please do not open follow-up issues automatically for now.

Reasoning:

  • None of the three deferred items are blockers for this PR.
  • They span different scopes: shared component defaults (Spinner / Banner), translation structure (<Trans> for Terms & Conditions), and a small DRY cleanup (PAST_DURATION_FILTER_I18N_KEYS). I do not want to bundle them into ad hoc issues without first deciding the right ownership and priority after this i18n PR lands.
  • I want to keep this PR narrowly focused on localized strings and the minimum code changes required to preserve upstream behavior.

I will track those separately later if they still matter after merge.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Tip

For best results, initiate chat on the files or code changes.

@xuhongbo — understood, I won't open issues for those items. The three deferred follow-ups (duplicate PAST_DURATION_FILTER_I18N_KEYS, Spinner/Banner English-only defaults, <Trans>-based Terms & Conditions) are noted and can be revisited after this PR lands whenever you decide on ownership and priority.

🐇 。 ✿


✏️ Learnings added
Learnt from: xuhongbo
URL: https://github.com/makeplane/plane/pull/9082

Timestamp: 2026-05-15T04:50:49.188Z
Learning: For the makeplane/plane repository, do not automatically open follow-up GitHub issues for deferred or out-of-scope review items. The team prefers to track those separately and decide ownership/priority after the relevant PR lands.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant