-
Notifications
You must be signed in to change notification settings - Fork 9
Implemented Internationalization #18
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
base: development
Are you sure you want to change the base?
Conversation
WalkthroughAdds next-intl internationalization: locale routing, middleware, request config, navigation helpers, provider wiring, English and Spanish message bundles, locale-aware layout and pages under [locale], header locale switcher, and replaces many hard-coded strings with useTranslations. Also wraps Next.js config with next-intl and removes the non-localized root home page. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant B as Browser
participant M as i18n Middleware
participant A as Next App Router
participant L as [locale]/RootLayout
participant P as Page Component
B->>M: Request /es/dashboard
M->>A: Forward request (locale inferred)
A->>L: Render layout with params.locale
L->>L: validate locale (routing.locales)
L->>L: getMessages(locale) / setRequestLocale
L->>P: Provide NextIntlClientProvider(messages)
P->>P: useTranslations('namespace') lookups
P-->>B: Localized HTML response
sequenceDiagram
autonumber
participant U as User
participant H as DashboardHeader
participant N as i18n Navigation
participant R as Next Router
U->>H: Open language dropdown
H->>U: Show locales (en, es)
U->>H: Select "es"
H->>N: getPathname()
H->>R: router.replace(pathname, { locale: "es" })
R-->>U: Page reloads/rerenders with locale=es
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/package.json (1)
3-3: Increment the package version per coding guidelines.The version field must be incremented by 0.0.1 with each change.
As per coding guidelines
Apply this diff:
- "version": "1.7.8", + "version": "1.7.9",apps/web/src/app/[locale]/dashboard/webhooks/page.tsx (1)
4-4: Use locale-aware navigation imports.Import
useRouterfrom the locale-aware navigation wrapper instead of Next.js directly to ensure navigation respects the current locale.Apply this diff:
-import { useRouter } from 'next/navigation'; +import { useRouter } from '@/i18n/navigation';apps/web/src/app/[locale]/dashboard/page.tsx (1)
4-60: Fix quick action links to preserve locale context.After moving dashboard routes under
[locale], linking to/dashboard/...strips the active locale, so visiting from/es/...kicks users back to the default locale (or 404s). Prefix these paths with the current locale (or switch to the localized navigation helper) so language selection survives navigation.-import { useTranslations } from 'next-intl'; +import { useLocale, useTranslations } from 'next-intl'; … - const t = useTranslations('dashboard'); + const t = useTranslations('dashboard'); + const locale = useLocale(); + const dashboardBasePath = `/${locale}/dashboard`; … - <Link href="/dashboard/webhooks"> + <Link href={`${dashboardBasePath}/webhooks`}> … - <Link href="/dashboard/send"> + <Link href={`${dashboardBasePath}/send`}> … - <Link href="/dashboard/templates"> + <Link href={`${dashboardBasePath}/templates`}>apps/web/src/app/[locale]/dashboard/send/page.tsx (1)
6-245: Keep settings link locale-aware.The toast link still points at
/dashboard/settings, which drops the[locale]segment you just introduced. In non-default locales this bounces users to the wrong language (or 404). Prefix the href with the active locale (or use the localizedLink).-import { useTranslations } from 'next-intl'; +import { useLocale, useTranslations } from 'next-intl'; … - const t = useTranslations('dashboard.sendMessagePage'); + const t = useTranslations('dashboard.sendMessagePage'); + const locale = useLocale(); … - <Link - href="/dashboard/settings" + <Link + href={`/${locale}/dashboard/settings`}apps/web/src/app/[locale]/dashboard/settings/page.tsx (1)
13-167: Preserve locale when linking to Plans.This button still targets
/dashboard/plans, omitting the locale segment. Users on/es/...will leave the Spanish experience (or miss the page). Prefix the path with the active locale (or use the locale-aware navigation helper).-import { useTranslations } from 'next-intl'; +import { useLocale, useTranslations } from 'next-intl'; … - const t = useTranslations('dashboard.settingsPage'); + const t = useTranslations('dashboard.settingsPage'); + const locale = useLocale(); … - <Link href="/dashboard/plans"> + <Link href={`/${locale}/dashboard/plans`}>
🧹 Nitpick comments (5)
apps/web/src/app/[locale]/page.tsx (1)
6-22: Consider translating or externalizing SEO keywords.The metadata title and description are correctly translated, but the keywords array is hardcoded. For better i18n coverage and maintainability, consider:
- Moving keywords to the translation files (for language-specific SEO)
- Or extracting them to a shared constants file if they're intentionally language-agnostic
If keywords should be translated, add them to your message files:
export const generateMetadata = async (): Promise<Metadata> => { const t = await getTranslations('meta'); return { title: t('title'), description: t('description'), - keywords: [ - 'Discord', - 'Webhook', - 'Manager', - 'Automation', - 'Messages', - 'Bots', - 'Communication', - 'Tools', - ], + keywords: t('keywords').split(',').map(k => k.trim()), }; };Then in your message files:
{ "meta": { "title": "...", "description": "...", "keywords": "Discord, Webhook, Manager, Automation, Messages, Bots, Communication, Tools" } }apps/web/src/i18n/request.ts (1)
14-14: Consider error handling for missing message files.While the locale is validated, there's no error handling for cases where the message file might be missing or malformed. Consider adding a try-catch block to provide better error messages during development.
return { locale, - messages: (await import(`../../messages/${locale}.json`)).default, + messages: await (async () => { + try { + return (await import(`../../messages/${locale}.json`)).default; + } catch (error) { + console.error(`Failed to load messages for locale "${locale}":`, error); + throw new Error(`Message file not found for locale: ${locale}`); + } + })(), };apps/web/src/app/[locale]/dashboard/templates/create/page.tsx (1)
26-29: Consider moving interface to a shared types file.The
TemplateFormRefinterface is defined locally. If this interface is used by the TemplateForm component, consider defining it in a shared types file or co-locating it with the component definition to improve maintainability.If the interface is specific to the TemplateForm component, consider exporting it from the component file:
// In template-form.tsx export interface TemplateFormRef { submit: () => void; }Then import it here:
import { TemplateForm, type TemplateFormRef } from '@/components/template-form';apps/web/src/app/[locale]/dashboard/avatars/page.tsx (2)
72-77: Remove excessive inline translation comments.The numerous inline comments explaining each translation (e.g., "// 3. Translate Title", "// 4. Translate Subtitle") are redundant. The code is self-documenting—seeing
t('title')clearly indicates a translation. These comments add noise without providing additional value.Remove the numbered translation comments:
<div> <h1 className="text-3xl font-bold text-white mb-2"> - {/* 3. Translate Title */} {t('title')} </h1> <p className="text-slate-400"> - {/* 4. Translate Subtitle */} {t('subtitle')} </p> </div>Keep only the meaningful comments that explain non-obvious decisions (like the one about the loading state or dialog translations).
Also applies to: 85-86, 94-95, 114-123, 131-132
105-107: Consider the loading message suggestion.The comment suggests adding a loading message, but showing only a spinner is a common and acceptable pattern. Adding a text message is optional and depends on your UX preferences.
If you decide to add one, ensure the 'loading' key exists in your message bundles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
apps/web/messages/en.json(1 hunks)apps/web/messages/es.json(1 hunks)apps/web/next.config.ts(2 hunks)apps/web/package.json(2 hunks)apps/web/src/app/[locale]/dashboard/avatars/page.tsx(5 hunks)apps/web/src/app/[locale]/dashboard/page.tsx(3 hunks)apps/web/src/app/[locale]/dashboard/plans/page.tsx(4 hunks)apps/web/src/app/[locale]/dashboard/send/page.tsx(24 hunks)apps/web/src/app/[locale]/dashboard/settings/page.tsx(6 hunks)apps/web/src/app/[locale]/dashboard/templates/create/page.tsx(4 hunks)apps/web/src/app/[locale]/dashboard/templates/page.tsx(12 hunks)apps/web/src/app/[locale]/dashboard/webhooks/page.tsx(10 hunks)apps/web/src/app/[locale]/layout.tsx(3 hunks)apps/web/src/app/[locale]/login/page.tsx(2 hunks)apps/web/src/app/[locale]/page.tsx(1 hunks)apps/web/src/app/page.tsx(0 hunks)apps/web/src/components/dashboard-header.tsx(3 hunks)apps/web/src/i18n/navigation.ts(1 hunks)apps/web/src/i18n/request.ts(1 hunks)apps/web/src/i18n/routing.ts(1 hunks)apps/web/src/middleware.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/app/page.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/web/**/package.json
📄 CodeRabbit inference engine (apps/web/GEMINI.md)
Increment the "version" field in package.json by 0.0.1 with each change (e.g., 1.0.0 -> 1.0.1)
Files:
apps/web/package.json
🧬 Code graph analysis (9)
apps/web/src/i18n/navigation.ts (1)
apps/web/src/i18n/routing.ts (1)
routing(8-14)
apps/web/src/components/dashboard-header.tsx (3)
apps/web/src/contexts/auth-context.tsx (1)
useAuth(85-91)packages/ui/src/button.tsx (1)
Button(11-20)apps/web/src/i18n/routing.ts (1)
localeMap(3-6)
apps/web/src/middleware.ts (1)
apps/web/src/i18n/routing.ts (1)
routing(8-14)
apps/web/src/i18n/request.ts (1)
apps/web/src/i18n/routing.ts (1)
routing(8-14)
apps/web/src/app/[locale]/dashboard/webhooks/page.tsx (1)
apps/web/src/contexts/auth-context.tsx (1)
useAuth(85-91)
apps/web/src/app/[locale]/layout.tsx (2)
apps/web/src/i18n/routing.ts (1)
routing(8-14)apps/web/src/lib/providers.tsx (1)
Providers(8-19)
apps/web/src/app/[locale]/login/page.tsx (2)
apps/web/src/lib/api/index.ts (1)
api(12-19)apps/web/src/components/discord-logo.tsx (1)
DiscordLogo(3-9)
apps/web/src/app/[locale]/dashboard/plans/page.tsx (1)
apps/web/src/components/settings/settings-card.tsx (1)
SettingsCard(20-53)
apps/web/src/app/[locale]/dashboard/settings/page.tsx (2)
apps/web/src/components/settings/settings-card.tsx (1)
SettingsCard(20-53)apps/web/src/components/ui/confirmation-dialog.tsx (1)
ConfirmationDialog(23-55)
🔇 Additional comments (28)
apps/web/package.json (1)
56-56: LGTM! next-intl dependency added correctly.The next-intl version ^4.3.11 aligns with the library's v4.x series and supports the App Router and Server Components features used throughout this PR.
Based on learnings
apps/web/next.config.ts (1)
2-4: LGTM! Standard next-intl plugin integration.The config wrapper correctly integrates next-intl at build time, enabling locale routing and message loading across the app.
Also applies to: 13-13
apps/web/src/i18n/navigation.ts (1)
1-7: LGTM! Locale-aware navigation wrappers correctly configured.These wrappers ensure that all navigation operations respect the locale routing configuration.
apps/web/src/app/[locale]/dashboard/webhooks/page.tsx (2)
57-59: Verify locale-aware navigation after fixing the import.After updating the import on Line 4, confirm that
router.push('/dashboard/send?webhookId=${webhook.id}')correctly navigates with locale awareness. The i18n-wrapped router should automatically preserve the locale in the path.
16-16: LGTM! Translations integrated correctly.All UI strings are properly externalized using the
useTranslationshook with appropriate keys from the dashboard namespace.Also applies to: 21-21, 66-68, 78-92, 99-112, 121-160, 201-215
apps/web/src/components/dashboard-header.tsx (1)
15-15: LGTM! Locale switcher properly implemented.The component correctly uses locale-aware navigation and implements a dropdown to switch between available locales. The
router.replacewithlocaleparameter ensures proper locale transitions.Also applies to: 25-32, 124-156
apps/web/messages/es.json (1)
1-252: LGTM! Comprehensive Spanish translations provided.The translation file covers all UI strings across the application with appropriate Spanish text.
apps/web/src/middleware.ts (1)
1-11: LGTM! Standard next-intl middleware setup.The middleware correctly integrates locale routing and excludes internal paths, API routes, and static assets from locale processing.
apps/web/src/i18n/routing.ts (1)
1-14: LGTM! Clean i18n routing configuration.The routing configuration correctly defines supported locales and sets a default, enabling locale-aware routing throughout the application.
apps/web/src/app/[locale]/login/page.tsx (4)
7-7: LGTM! Clean i18n integration.The useTranslations hook is properly imported and initialized with the 'login' namespace, matching the structure defined in the message bundles.
Also applies to: 11-11
19-19: LGTM! Simplified error handling.Catching without the error parameter is appropriate here since the error is not used. This simplifies the code while maintaining the same error-handling behavior.
24-24: LGTM! Correct dependency array.Adding
routerto the dependency array is correct. The router fromnext/navigationis stable and won't cause unnecessary re-renders.
34-34: LGTM! Consistent translation usage.All user-facing strings are properly replaced with translation keys from the 'login' namespace. The keys match the structure in the message bundles.
Also applies to: 42-43, 49-49
apps/web/src/app/[locale]/layout.tsx (3)
15-17: LGTM! Proper static generation setup.The generateStaticParams function correctly enumerates all supported locales from the routing configuration for static generation.
49-56: LGTM! Robust locale handling.The locale validation with
hasLocaleand fallback tonotFound()ensures only valid locales are processed. The message loading and locale setting follow next-intl best practices.
75-77: LGTM! Proper provider nesting.The NextIntlClientProvider is correctly positioned to wrap the app's providers, ensuring translations are available throughout the component tree.
apps/web/src/app/[locale]/page.tsx (2)
29-127: LGTM! Comprehensive translation usage.All user-facing strings are properly translated, including:
- Hero section content
- Feature descriptions (with array iteration)
- Contact information
- Footer text
The conditional rendering based on login state is well-handled with appropriate translation keys.
25-27: Cookie-based login detection verified. TherefreshTokencookie is set in/api/auth/set-refresh-token, refreshed in/api/auth/refresh-token, and cleared in/api/auth/logout. No unhandled cases or race conditions were found.apps/web/messages/en.json (2)
1-252: LGTM! Well-structured localization resource.The English message bundle is comprehensive and well-organized with:
- Clear namespace hierarchy (meta, home, login, dashboard with subsections)
- Proper ICU MessageFormat syntax for pluralization
- Consistent variable interpolation patterns
- Complete coverage of UI strings across multiple pages
The structure aligns well with the component usage patterns seen in the other files.
108-108: Adopt proper ICU pluralization syntax
The current"embedCount": "{count} embed{plural}"string does not use the ICU pattern; update it to, for example:"embedCount": "{count, plural, one {# embed} other {# embeds}}"[next-intl v4.3.11 supports this syntax]
apps/web/src/i18n/request.ts (1)
5-16: LGTM! Secure locale resolution with proper validation.The implementation correctly:
- Validates the requested locale using
hasLocalebefore any file operations- Falls back to
defaultLocalefor invalid locales- Uses validated locale values in the dynamic import path, preventing path traversal
The locale validation before the dynamic import ensures that only expected message files can be loaded.
apps/web/src/app/[locale]/dashboard/templates/page.tsx (3)
42-42: LGTM! Consistent translation integration.The translation hook is properly initialized and used consistently throughout the component for all user-facing strings including titles, subtitles, buttons, and form inputs.
Also applies to: 61-62, 90-92, 99-99
81-83: LGTM! Clean handler simplification.Converting the navigation handlers to concise arrow functions improves readability while maintaining the same functionality.
251-253: LGTM! Proper string interpolation with translations.The template name is correctly interpolated into the translated dialog description using next-intl's parameter substitution.
apps/web/src/app/[locale]/dashboard/templates/create/page.tsx (2)
17-17: LGTM! Proper translation setup.The useTranslations hook is correctly imported and initialized with the 'dashboard.createTemplate' namespace.
Also applies to: 20-20
87-103: LGTM! Clean conditional translation usage.The conditional rendering for create vs. edit mode correctly uses different translation keys, and the button states are properly handled with translated labels for each state.
apps/web/src/app/[locale]/dashboard/avatars/page.tsx (2)
7-7: LGTM! Proper translation setup.The useTranslations hook is correctly imported and initialized with the 'dashboard.avatarsPage' namespace.
Also applies to: 20-20
115-123: LGTM! Smart conditional empty state rendering.The empty state correctly switches between "no results" and "no avatars" messages based on the search query state, providing appropriate user feedback for each scenario.
|
Someone is attempting to deploy a commit to the Ctrix Projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Done changes suggested by codderabbit |
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: 3
🧹 Nitpick comments (5)
apps/web/src/components/dashboard-header.tsx (2)
130-131: Localize the language button label.Avoid hardcoded "Lang". Show the current locale’s native name instead.
- Lang ({locale}) + {localeMap[locale as keyof typeof localeMap]}
133-149: Preserve query string when switching locales (optional).router.replace(pathname, { locale: loc }) drops ?query/#hash. Consider preserving search params (e.g., read via useSearchParams and re-append) or compute the target with getPathname and include the current search string.
apps/web/messages/es.json (2)
182-182: Translate “Embeds” tab label.Use a Spanish term to stay consistent with “incrustación”.
- "embeds": "Embeds", + "embeds": "Incrustaciones",
103-103: Replace placeholder “TODO usos” or make it dynamic (optional).Either provide a real translation (e.g., "Usos") or accept a {count} to show usage when available.
apps/web/src/app/[locale]/dashboard/templates/page.tsx (1)
199-200: Localize date formatting.Use next-intl’s useFormatter() for locale-aware dates instead of toLocaleDateString() without a locale.
Example:
const fmt = useFormatter(); fmt.date(new Date(template.updatedAt), { dateStyle: 'medium' });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/messages/es.json(1 hunks)apps/web/src/app/[locale]/dashboard/templates/page.tsx(12 hunks)apps/web/src/app/[locale]/layout.tsx(3 hunks)apps/web/src/components/dashboard-header.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/app/[locale]/layout.tsx (2)
apps/web/src/i18n/routing.ts (1)
routing(8-14)apps/web/src/lib/providers.tsx (1)
Providers(8-19)
apps/web/src/components/dashboard-header.tsx (3)
apps/web/src/contexts/auth-context.tsx (1)
useAuth(85-91)packages/ui/src/button.tsx (1)
Button(11-20)apps/web/src/i18n/routing.ts (1)
localeMap(3-6)
🔇 Additional comments (2)
apps/web/src/components/dashboard-header.tsx (1)
15-16: Verify locale-aware navigations for absolute pushes.Ensure router.push('/dashboard/webhooks') and router.push('/') resolve to the correct locale-prefixed routes after i18n. If needed, pass { locale } or use Link from the i18n navigation helpers to guarantee correct prefixes.
Based on learnings
Also applies to: 123-150
apps/web/src/app/[locale]/dashboard/templates/page.tsx (1)
193-195: Ensure message uses ICU pluralization for embedCount.You updated the call to pass only {count}. Confirm the corresponding en/es messages use ICU plural syntax to avoid missing-arg errors (see suggested change in apps/web/messages/es.json).
| params: Promise<{ locale: string }>; | ||
| }>) { | ||
| const { locale } = await params; | ||
| if (!hasLocale(routing.locales, locale)) { |
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.
Fix params typing and set locale before loading messages.
- params is not a Promise in App Router. Type it as a plain object and remove unnecessary await.
- Call setRequestLocale(locale) before getMessages() to ensure messages for the active locale are loaded.
- params: Promise<{ locale: string }>;
+ params: { locale: string };
}) {
- const { locale } = await params;
+ const { locale } = params;- const messages = await getMessages();
-
- setRequestLocale(locale);
+ setRequestLocale(locale);
+ const messages = await getMessages();Based on learnings
Also applies to: 54-57
🤖 Prompt for AI Agents
In apps/web/src/app/[locale]/layout.tsx around lines 47-50 (and similarly
54-57), params is incorrectly typed/treated as a Promise and locale is set after
loading messages; change the function signature to accept params as a plain
object (e.g., params: { locale: string }), remove the await when extracting
locale (const { locale } = params), and call setRequestLocale(locale) before
invoking getMessages() so the correct locale messages are loaded (apply the same
fixes to the second occurrence at lines 54-57).
|
still some left @mdrehan369 |
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: 1
🧹 Nitpick comments (1)
apps/web/src/app/[locale]/layout.tsx (1)
74-76: Consider passing locale prop to NextIntlClientProvider for clarity.While
setRequestLocale(locale)makes the locale implicitly available toNextIntlClientProvider, explicitly passing thelocaleprop improves clarity and type safety.Apply this diff to pass the locale prop explicitly:
- <NextIntlClientProvider messages={messages}> + <NextIntlClientProvider locale={locale} messages={messages}> <Providers>{children}</Providers> </NextIntlClientProvider>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/messages/es.json(1 hunks)apps/web/src/app/[locale]/dashboard/templates/page.tsx(12 hunks)apps/web/src/app/[locale]/layout.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/messages/es.json
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/[locale]/layout.tsx (2)
apps/web/src/i18n/routing.ts (1)
routing(8-14)apps/web/src/lib/providers.tsx (1)
Providers(8-19)
🔇 Additional comments (4)
apps/web/src/app/[locale]/dashboard/templates/page.tsx (3)
4-4: LGTM! Past review comments addressed.The router import has been correctly updated to use the i18n-aware navigation wrapper, and the useTranslations hook is properly configured with the dashboard.templates namespace.
Also applies to: 42-42
61-62: Excellent i18n implementation!All translation keys are consistently applied throughout the component, covering titles, buttons, placeholders, dialog content, and fallback messages. The dynamic interpolation in the delete dialog description correctly passes the template name.
Also applies to: 90-92, 99-99, 108-108, 137-137, 157-157, 164-164, 179-179, 184-184, 212-212, 215-215, 222-222, 231-231, 234-234, 247-247, 250-252, 257-257, 266-266
81-83: Router usage is now locale-aware.The router.push calls will automatically prefix routes with the current locale since you're using the i18n navigation wrapper. No further changes needed.
apps/web/src/app/[locale]/layout.tsx (1)
42-55: Previous issues resolved; params typing is correct for Next.js 15.The past review comments have been addressed:
- The
langattribute now uses the validatedlocalevariable (line 58).setRequestLocale(locale)is called beforegetMessages()(lines 54-55), ensuring the correct locale messages are loaded.Regarding the
paramstyping: In Next.js 15 with async Server Components,paramscan be aPromise, and the current implementation (params: Promise<{ locale: string }>withawait params) is correct and aligns with Next.js 15 patterns. The earlier comment suggesting a plain object was based on older Next.js conventions and does not apply here.
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: 2
🧹 Nitpick comments (3)
apps/web/src/components/settings/theme-selector.tsx (1)
22-25: Consider removing redundant JSX comments.The translation usage is correct, but the inline JSX comments are self-explanatory and could be removed for cleaner code.
Apply this diff to remove the redundant comments:
- {/* Translate Title */} <Label className="text-base font-medium text-white">{t('title')}</Label> - {/* Translate Description */} <p className="text-sm text-slate-300 mb-3">{t('description')}</p> <div className="grid grid-cols-3 gap-3"> {themes.map(({ value, labelKey, icon: Icon }) => ( <Button key={value} variant={theme === value ? 'default' : 'outline'} onClick={() => setTheme(value)} className="flex items-center gap-2 h-12 bg-slate-800/50 border-slate-600 hover:bg-slate-700/50 text-white" > <Icon className="h-4 w-4" /> - {/* Translate Label */} {t(labelKey)} </Button>Also applies to: 35-36
apps/web/src/components/avatars/avatar-card.tsx (1)
179-181: Use next-intl's date formatter for locale consistency.The current implementation uses
toLocaleDateString(), which formats the date according to the browser's locale rather than the selected next-intl locale. This can create inconsistency between the UI language and date formatting.Apply this diff to use next-intl's date formatting:
+import { useFormatter } from 'next-intl'; export function AvatarCard({ avatar, onEdit, onDeleteSuccess, onSelect, selectable = false, onCardClick, }: AvatarCardProps) { const t = useTranslations('avatarCard'); // Initialize translations + const format = useFormatter(); const [showDeleteDialog, setShowDeleteDialog] = useState(false);Then update the date formatting:
<div className="text-xs text-slate-500 mb-3"> {/* Translate Created date label and format the date */} {t('info.created', { - date: new Date(avatar.createdAt).toLocaleDateString(), + date: format.dateTime(new Date(avatar.createdAt), { + year: 'numeric', + month: 'short', + day: 'numeric' + }), })} </div>Based on learnings
apps/web/src/components/template-form.tsx (1)
82-93: Consider explicit handling when avatar_ref doesn't match any fetched avatar.The current implementation silently keeps the current state when
avatar_refexists but doesn't match any fetched avatar (e.g., if the referenced avatar was deleted). This could confuse users who expect to see a specific avatar but see the default instead.Consider adding explicit feedback or logging:
const matchedAvatar = avatars.find( a => a.id === initialData.avatar_ref ); if (matchedAvatar) { setSelectedAvatar(matchedAvatar); } else { - // Handle case where avatar_ref exists but doesn't match an fetched avatar - // For now, reset to default or keep current state + // Handle case where avatar_ref exists but doesn't match any fetched avatar + // Reset to default avatar and optionally log or notify + console.warn(`Avatar with ID ${initialData.avatar_ref} not found, using default avatar`); + // Could also show a toast notification here if desired }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/messages/en.json(1 hunks)apps/web/messages/es.json(1 hunks)apps/web/src/app/[locale]/dashboard/webhooks/page.tsx(10 hunks)apps/web/src/components/add-webhook-dialog.tsx(5 hunks)apps/web/src/components/avatars/avatar-card.tsx(8 hunks)apps/web/src/components/avatars/avatar-selector.tsx(8 hunks)apps/web/src/components/avatars/create-avatar-dialog.tsx(12 hunks)apps/web/src/components/dashboard-header.tsx(5 hunks)apps/web/src/components/embed-builder.tsx(23 hunks)apps/web/src/components/settings/theme-selector.tsx(1 hunks)apps/web/src/components/template-form.tsx(7 hunks)apps/web/src/components/webhook-card.tsx(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/messages/es.json
- apps/web/src/app/[locale]/dashboard/webhooks/page.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
apps/web/src/components/add-webhook-dialog.tsx (1)
packages/ui/src/button.tsx (1)
Button(11-20)
apps/web/src/components/avatars/create-avatar-dialog.tsx (4)
apps/web/src/hooks/use-toast.ts (1)
toast(184-184)apps/web/src/components/ui/dialog.tsx (2)
DialogTitle(141-141)DialogHeader(138-138)apps/web/src/components/ui/label.tsx (1)
Label(24-24)apps/web/src/components/ui/input.tsx (1)
Input(21-21)
apps/web/src/components/dashboard-header.tsx (2)
apps/web/src/contexts/auth-context.tsx (1)
useAuth(85-91)apps/web/src/i18n/routing.ts (1)
localeMap(3-6)
apps/web/src/components/embed-builder.tsx (1)
packages/ui/src/button.tsx (1)
Button(11-20)
apps/web/src/components/avatars/avatar-card.tsx (2)
apps/web/src/hooks/use-toast.ts (1)
toast(184-184)packages/ui/src/button.tsx (1)
Button(11-20)
🔇 Additional comments (17)
apps/web/src/components/settings/theme-selector.tsx (1)
3-3: LGTM: Proper next-intl integration.The import and hook initialization follow next-intl best practices. Using a namespaced approach ('themeSelector') for translations is recommended for type safety and maintainability.
Based on learnings.
Also applies to: 10-10
apps/web/src/components/add-webhook-dialog.tsx (1)
25-163: Solid i18n integrationScoped translations cover trigger, dialog, form, and action text cleanly without altering behavior. Looks good.
apps/web/src/components/avatars/avatar-selector.tsx (1)
32-144: Nice localization passAll user-facing strings now come from the avatarSelector namespace and the existing UX stays intact. 👍
apps/web/src/components/webhook-card.tsx (1)
40-271: Comprehensive translation coverageStatus badges, dropdown actions, dialogs, and toasts now respect locale strings while preserving behavior. Well done.
apps/web/src/components/embed-builder.tsx (1)
23-583: Thorough embed builder localizationEvery label, helper, and CTA is now driven by embedBuilder translations without touching business logic. Looks great.
apps/web/src/components/dashboard-header.tsx (1)
29-193: Great header internationalizationTitle/subtitle, language switcher, account badges, and menu items all source translations correctly while keeping interactions the same.
apps/web/src/components/avatars/avatar-card.tsx (4)
5-5: LGTM! Translation setup is correct.The import and initialization of
useTranslationswith the'avatarCard'namespace follows next-intl best practices.Also applies to: 51-51
147-170: LGTM! Dropdown action translations are correctly implemented.All dropdown menu items (Edit, Copy URL, Delete) are properly translated with appropriate translation keys.
189-191: LGTM! Dialog and button translations are correctly implemented.The select button, delete dialog title, description with username interpolation, and action buttons are all properly translated.
Also applies to: 200-218
60-73: All translation keys exist. Verified thatavatarCard.toastincludesdeleteSuccessTitle,deleteSuccessDescription, anddeleteErrorTitlein bothapps/web/messages/en.jsonandapps/web/messages/es.json.apps/web/src/components/template-form.tsx (7)
4-4: LGTM! Translation setup is correct.The import and initialization of
useTranslationswith the'templateForm'namespace follows next-intl best practices.Also applies to: 42-42
101-101: LGTM! Dependency array correctly includes 'avatars'.Adding
avatarsto the dependency array ensures that the avatar selection logic re-runs when the avatars data changes, which is necessary for the new avatar matching logic to work correctly.
115-133: LGTM! Tab translations are correctly implemented.All three tabs (Info, Message, Embeds) are properly translated with appropriate translation keys.
141-175: LGTM! Info tab translations are correctly implemented.All labels, placeholders, and headers in the info tab are properly translated.
185-209: LGTM! Message content translations are correctly implemented.All headers, labels, placeholders, and character count with interpolation are properly translated.
215-231: LGTM! Avatar section translations are correctly implemented.The avatar header and select button are properly translated.
42-42: All 'templateForm' translation keys verified
All keys (infoTab, messageTab, tabs) exist in bothapps/web/messages/en.jsonandapps/web/messages/es.json.
| "delete": "Eliminar", | ||
| "areYouSure": "¿Estás seguro?" | ||
| }, | ||
| "webhookCard": { | ||
| "status": { | ||
| "active": "Activo", | ||
| "inactive": "Inactivo" | ||
| }, | ||
| "details": { | ||
| "description": "Descripción:", | ||
| "messagesSent": "Mensajes enviados:", | ||
| "created": "Creado:", | ||
| "lastUsed": "Último uso:", | ||
| "todoPlaceholder": "PENDIENTE" | ||
| }, | ||
| "actions": { | ||
| "testWebhook": "Probar Webhook", | ||
| "testing": "Probando...", | ||
| "deactivate": "Desactivar", | ||
| "activate": "Activar", | ||
| "delete": "Eliminar" | ||
| }, | ||
| "dialog": { | ||
| "title": "¿Estás seguro?", | ||
| "description": "Esta acción no se puede deshacer. Esto eliminará permanentemente el webhook \"{webhookName}\" de tu cuenta." | ||
| }, | ||
| "toasts": { | ||
| "updateSuccessTitle": "Webhook actualizado", | ||
| "updateSuccessDesc": "Webhook actualizado correctamente", | ||
| "updateErrorTitle": "Error al actualizar el webhook", | ||
| "deleteSuccessTitle": "Webhook eliminado", | ||
| "deleteSuccessDesc": "El webhook ha sido eliminado de tu cuenta", | ||
| "testSuccessTitle": "¡Prueba exitosa!", | ||
| "testSuccessDesc": "Mensaje de prueba enviado a Discord", | ||
| "testFailTitle": "Prueba fallida", | ||
| "testFailGenericDesc": "Ocurrió un error al probar el webhook" | ||
| } | ||
| } |
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.
Fix Spanish text in the English locale
Entries under common and webhookCard are Spanish (“Eliminar”, “¿Estás seguro?”, “Activo”, …). With the English locale active, UI strings like the delete confirmation dialog and webhook status will appear in Spanish instead of English. Please replace these values with their English equivalents (e.g., “Delete”, “Are you sure?”, “Active”, “Inactive”, etc.) so the default locale renders correctly.
- "delete": "Eliminar",
- "areYouSure": "¿Estás seguro?"
+ "delete": "Delete",
+ "areYouSure": "Are you sure?"
},
"webhookCard": {
"status": {
- "active": "Activo",
- "inactive": "Inactivo"
+ "active": "Active",
+ "inactive": "Inactive"
},
"details": {
- "description": "Descripción:",
- "messagesSent": "Mensajes enviados:",
- "created": "Creado:",
- "lastUsed": "Último uso:",
- "todoPlaceholder": "PENDIENTE"
+ "description": "Description:",
+ "messagesSent": "Messages sent:",
+ "created": "Created:",
+ "lastUsed": "Last used:",
+ "todoPlaceholder": "TODO"
},
"actions": {
- "testWebhook": "Probar Webhook",
- "testing": "Probando...",
- "deactivate": "Desactivar",
- "activate": "Activar",
- "delete": "Eliminar"
+ "testWebhook": "Test webhook",
+ "testing": "Testing...",
+ "deactivate": "Deactivate",
+ "activate": "Activate",
+ "delete": "Delete"
},
"dialog": {
- "title": "¿Estás seguro?",
- "description": "Esta acción no se puede deshacer. Esto eliminará permanentemente el webhook \"{webhookName}\" de tu cuenta."
+ "title": "Are you sure?",
+ "description": "This action cannot be undone. It will permanently delete the webhook \"{webhookName}\" from your account."
},
"toasts": {
- "updateSuccessTitle": "Webhook actualizado",
- "updateSuccessDesc": "Webhook actualizado correctamente",
- "updateErrorTitle": "Error al actualizar el webhook",
- "deleteSuccessTitle": "Webhook eliminado",
- "deleteSuccessDesc": "El webhook ha sido eliminado de tu cuenta",
- "testSuccessTitle": "¡Prueba exitosa!",
- "testSuccessDesc": "Mensaje de prueba enviado a Discord",
- "testFailTitle": "Prueba fallida",
- "testFailGenericDesc": "Ocurrió un error al probar el webhook"
+ "updateSuccessTitle": "Webhook updated",
+ "updateSuccessDesc": "Webhook updated successfully",
+ "updateErrorTitle": "Error updating webhook",
+ "deleteSuccessTitle": "Webhook deleted",
+ "deleteSuccessDesc": "The webhook has been removed from your account",
+ "testSuccessTitle": "Test successful!",
+ "testSuccessDesc": "Test message sent to Discord",
+ "testFailTitle": "Test failed",
+ "testFailGenericDesc": "An error occurred while testing the webhook"
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "delete": "Eliminar", | |
| "areYouSure": "¿Estás seguro?" | |
| }, | |
| "webhookCard": { | |
| "status": { | |
| "active": "Activo", | |
| "inactive": "Inactivo" | |
| }, | |
| "details": { | |
| "description": "Descripción:", | |
| "messagesSent": "Mensajes enviados:", | |
| "created": "Creado:", | |
| "lastUsed": "Último uso:", | |
| "todoPlaceholder": "PENDIENTE" | |
| }, | |
| "actions": { | |
| "testWebhook": "Probar Webhook", | |
| "testing": "Probando...", | |
| "deactivate": "Desactivar", | |
| "activate": "Activar", | |
| "delete": "Eliminar" | |
| }, | |
| "dialog": { | |
| "title": "¿Estás seguro?", | |
| "description": "Esta acción no se puede deshacer. Esto eliminará permanentemente el webhook \"{webhookName}\" de tu cuenta." | |
| }, | |
| "toasts": { | |
| "updateSuccessTitle": "Webhook actualizado", | |
| "updateSuccessDesc": "Webhook actualizado correctamente", | |
| "updateErrorTitle": "Error al actualizar el webhook", | |
| "deleteSuccessTitle": "Webhook eliminado", | |
| "deleteSuccessDesc": "El webhook ha sido eliminado de tu cuenta", | |
| "testSuccessTitle": "¡Prueba exitosa!", | |
| "testSuccessDesc": "Mensaje de prueba enviado a Discord", | |
| "testFailTitle": "Prueba fallida", | |
| "testFailGenericDesc": "Ocurrió un error al probar el webhook" | |
| } | |
| } | |
| "delete": "Delete", | |
| "areYouSure": "Are you sure?" | |
| }, | |
| "webhookCard": { | |
| "status": { | |
| "active": "Active", | |
| "inactive": "Inactive" | |
| }, | |
| "details": { | |
| "description": "Description:", | |
| "messagesSent": "Messages sent:", | |
| "created": "Created:", | |
| "lastUsed": "Last used:", | |
| "todoPlaceholder": "TODO" | |
| }, | |
| "actions": { | |
| "testWebhook": "Test webhook", | |
| "testing": "Testing...", | |
| "deactivate": "Deactivate", | |
| "activate": "Activate", | |
| "delete": "Delete" | |
| }, | |
| "dialog": { | |
| "title": "Are you sure?", | |
| "description": "This action cannot be undone. It will permanently delete the webhook \"{webhookName}\" from your account." | |
| }, | |
| "toasts": { | |
| "updateSuccessTitle": "Webhook updated", | |
| "updateSuccessDesc": "Webhook updated successfully", | |
| "updateErrorTitle": "Error updating webhook", | |
| "deleteSuccessTitle": "Webhook deleted", | |
| "deleteSuccessDesc": "The webhook has been removed from your account", | |
| "testSuccessTitle": "Test successful!", | |
| "testSuccessDesc": "Test message sent to Discord", | |
| "testFailTitle": "Test failed", | |
| "testFailGenericDesc": "An error occurred while testing the webhook" | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/messages/en.json around lines 441 to 478, several values under
"common" and "webhookCard" are Spanish; replace them with English equivalents so
the English locale renders correctly. Update keys: common.delete -> "Delete",
common.areYouSure -> "Are you sure?", webhookCard.status.active -> "Active",
webhookCard.status.inactive -> "Inactive"; webhookCard.details.description ->
"Description:", messagesSent -> "Messages sent:", created -> "Created:",
lastUsed -> "Last used:", todoPlaceholder -> "TODO";
webhookCard.actions.testWebhook -> "Test Webhook", testing -> "Testing...",
deactivate -> "Deactivate", activate -> "Activate", delete -> "Delete";
dialog.title -> "Are you sure?", dialog.description -> 'This action cannot be
undone. This will permanently delete the "{webhookName}" webhook from your
account.'; toasts: updateSuccessTitle -> "Webhook updated", updateSuccessDesc ->
"Webhook updated successfully", updateErrorTitle -> "Error updating webhook",
deleteSuccessTitle -> "Webhook deleted", deleteSuccessDesc -> "The webhook has
been removed from your account", testSuccessTitle -> "Test successful!",
testSuccessDesc -> "Test message sent to Discord", testFailTitle -> "Test
failed", testFailGenericDesc -> "An error occurred while testing the webhook".
Ensure punctuation and casing match project conventions.
| // Map theme values to their respective translation keys and icons | ||
| const themes = [ | ||
| { value: 'light', label: 'Light', icon: Sun }, | ||
| { value: 'dark', label: 'Dark', icon: Moon }, | ||
| { value: 'system', label: 'System', icon: Monitor }, | ||
| { value: 'light', labelKey: 'themes.light', icon: Sun }, | ||
| { value: 'dark', labelKey: 'themes.dark', icon: Moon }, | ||
| { value: 'system', labelKey: 'themes.system', icon: Monitor }, | ||
| ]; |
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.
🧩 Analysis chain
Verify translation keys exist in message files.
The themes array structure is correct for internationalization. Ensure the following keys are defined in your themeSelector namespace message files:
themes.lightthemes.darkthemes.system
Run the following script to verify the translation keys exist:
🏁 Script executed:
#!/bin/bash
# Description: Verify translation keys exist in message files for themeSelector namespace.
# Search for message files containing themeSelector namespace
echo "Searching for themeSelector message definitions..."
rg -n --type=json -A 5 '"themeSelector"' -g '**/messages/**' -g '**/locales/**' -g '**/i18n/**'
# Also check for the specific theme keys
echo -e "\nSearching for theme translation keys..."
rg -n --type=json '"themes"\s*:' -g '**/messages/**' -g '**/locales/**' -g '**/i18n/**'Length of output: 907
Add missing themes.system key to themeSelector translations
apps/web/messages/en.json and apps/web/messages/es.json lack "system" under themeSelector.themes; please add "system": "System" (and localized value) to both.
🤖 Prompt for AI Agents
In apps/web/src/components/settings/theme-selector.tsx around lines 13 to 18 the
component references a translation key themes.system but the translation files
are missing that entry; open apps/web/messages/en.json and
apps/web/messages/es.json and under the themeSelector.themes object add a
"system" key with the appropriate text ("System" for en.json and the Spanish
localized string for es.json) so the lookup succeeds and the UI shows the
correct label.
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/template-form.tsx (1)
77-102: Prevent avatar query effect from wiping user input.When
initialDatais falsy (creating a new template), this effect now also runs whenever the avatars query resolves, so theelsebranch fires again and clears every field. Any content a user typed before the avatars arrive gets erased—a pretty painful regression. Please guard the reset logic so avatar updates don’t blast in-progress edits (e.g. run the population block only wheninitialDataexists, and move the reset into a separate effect keyed solely oninitialData).Apply this diff:
- useEffect(() => { - if (initialData) { - setName(initialData.name); - setDescription(initialData.description || ''); - setContent(initialData.content); - // Logic to set the selected avatar based on initialData.avatar_ref - if (avatars && avatars.length > 0) { - const matchedAvatar = avatars.find( - a => a.id === initialData.avatar_ref - ); - if (matchedAvatar) { - setSelectedAvatar(matchedAvatar); - } else { - // Handle case where avatar_ref exists but doesn't match an fetched avatar - // For now, reset to default or keep current state - } - } - setEmbeds(initialData.embeds || []); - } else { - setName(''); - setDescription(''); - setContent(''); - setEmbeds([]); - } - }, [initialData, avatars]); // Added 'avatars' to dependency array + useEffect(() => { + if (!initialData) return; + + setName(initialData.name); + setDescription(initialData.description || ''); + setContent(initialData.content); + + if (avatars?.length) { + const matchedAvatar = avatars.find( + avatar => avatar.id === initialData.avatar_ref + ); + if (matchedAvatar) { + setSelectedAvatar(matchedAvatar); + } + } + + setEmbeds(initialData.embeds || []); + }, [initialData, avatars]); + + useEffect(() => { + if (initialData) return; + setName(''); + setDescription(''); + setContent(''); + setEmbeds([]); + }, [initialData]);
🧹 Nitpick comments (4)
apps/web/src/components/avatars/avatar-card.tsx (1)
179-181: Consider using locale-aware date formatting.While
.toLocaleDateString()will use the browser's locale by default, explicitly passing the current locale from next-intl would ensure consistency with the selected app locale and provide better control over date formatting.You can use
useLocale()from next-intl and pass it to the date formatter:+'use client'; + +import { useLocale } from 'next-intl'; + export function AvatarCard({ avatar, onEdit, onDeleteSuccess, onSelect, selectable = false, onCardClick, }: AvatarCardProps) { const t = useTranslations('avatarCard'); + const locale = useLocale(); // ... rest of code // Later in the JSX: - date: new Date(avatar.createdAt).toLocaleDateString(), + date: new Date(avatar.createdAt).toLocaleDateString(locale),Alternatively, use next-intl's
useFormatter()hook for more advanced date formatting:const format = useFormatter(); // Then use: format.dateTime(new Date(avatar.createdAt), { dateStyle: 'short' })Based on learnings.
apps/web/src/components/avatars/avatar-selector.tsx (1)
45-55: Optional: Simplify redundant filtering logic.The filtering logic checks
avatar.usernameagainstsearchQuerytwice identically (lines 48-50 and 51-53). While the comment acknowledges this, you could simplify it for better maintainability.Apply this diff to simplify:
const filteredAvatars = avatars.filter((avatar: Avatar) => { - // The original logic checks username against itself twice, we'll keep the - // filtering logic as is, focusing on translation of visible text. - const nameMatches = - avatar.username && - avatar.username.toLowerCase().includes(searchQuery.toLowerCase()); - const usernameMatches = - avatar.username && - avatar.username.toLowerCase().includes(searchQuery.toLowerCase()); - return nameMatches || usernameMatches; + return ( + avatar.username && + avatar.username.toLowerCase().includes(searchQuery.toLowerCase()) + ); });apps/web/src/components/webhook-card.tsx (1)
246-247: Consider using locale-aware date formatting.The date formatting methods (
.toLocaleDateString()and.toLocaleString()) are called without explicit locale parameters. For consistency with the selected app locale, consider using next-intl's formatting utilities.Apply this diff to use locale-aware formatting:
+'use client'; + +import { useLocale } from 'next-intl'; + export function WebhookCard({ webhook, onWebhookUpdated, onCardClick, }: WebhookCardProps) { const t = useTranslations('webhookCard'); const tCommon = useTranslations('common'); + const locale = useLocale(); // ... rest of code // In the JSX: - {new Date(webhook.createdAt).toLocaleDateString()} + {new Date(webhook.createdAt).toLocaleDateString(locale)} // And: - {new Date(webhook.last_used).toLocaleString()} + {new Date(webhook.last_used).toLocaleString(locale)}Alternatively, use next-intl's
useFormatter()hook for more advanced and consistent date/time formatting:const format = useFormatter(); // Then use: // format.dateTime(new Date(webhook.createdAt), { dateStyle: 'short' }) // format.dateTime(new Date(webhook.last_used), { dateStyle: 'short', timeStyle: 'short' })Based on learnings.
Also applies to: 253-254
apps/web/src/components/embed-builder.tsx (1)
96-112: Consider reducing inline comment verbosity.The inline comments explaining each translation (e.g.,
/* Translate Header Title */) are helpful during development but add significant verbosity to the production code. The translation key names themselves are generally self-documenting (e.g.,t('header.title'),t('addButton')).Consider removing or consolidating these comments to improve code readability and reduce maintenance overhead.
For example, instead of:
- {/* Translate Header Title */} - <Label className="text-slate-200 font-medium"> - {t('header.title')} - </Label> - {/* Translate Header Subtitle, passing max value */} - <p className="text-sm text-slate-400"> - {t('header.subtitle', { max: DISCORD_MAX_EMBEDS })} - </p> + <Label className="text-slate-200 font-medium"> + {t('header.title')} + </Label> + <p className="text-sm text-slate-400"> + {t('header.subtitle', { max: DISCORD_MAX_EMBEDS })} + </p>This would reduce the file from 590 lines to approximately 530 lines while maintaining the same functionality.
Also applies to: 118-121, 131-142, 149-156, 168-175, 189-195, 209-212, 229-231, 266-267, 283-284, 293-302, 317-326, 341-348, 366-376, 386-399, 403-412, 424-432, 455-457, 468-475, 488-495, 509-544, 562-564
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/messages/en.json(1 hunks)apps/web/messages/es.json(1 hunks)apps/web/src/app/[locale]/dashboard/webhooks/page.tsx(10 hunks)apps/web/src/components/add-webhook-dialog.tsx(5 hunks)apps/web/src/components/avatars/avatar-card.tsx(8 hunks)apps/web/src/components/avatars/avatar-selector.tsx(8 hunks)apps/web/src/components/avatars/create-avatar-dialog.tsx(12 hunks)apps/web/src/components/dashboard-header.tsx(5 hunks)apps/web/src/components/embed-builder.tsx(23 hunks)apps/web/src/components/settings/theme-selector.tsx(1 hunks)apps/web/src/components/template-form.tsx(7 hunks)apps/web/src/components/webhook-card.tsx(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/messages/en.json
🧰 Additional context used
🧬 Code graph analysis (5)
apps/web/src/components/dashboard-header.tsx (2)
apps/web/src/contexts/auth-context.tsx (1)
useAuth(85-91)apps/web/src/i18n/routing.ts (1)
localeMap(3-6)
apps/web/src/components/embed-builder.tsx (1)
packages/ui/src/button.tsx (1)
Button(11-20)
apps/web/src/components/avatars/create-avatar-dialog.tsx (4)
apps/web/src/hooks/use-toast.ts (1)
toast(184-184)apps/web/src/components/ui/dialog.tsx (2)
DialogTitle(141-141)DialogHeader(138-138)apps/web/src/components/ui/label.tsx (1)
Label(24-24)apps/web/src/components/ui/input.tsx (1)
Input(21-21)
apps/web/src/components/add-webhook-dialog.tsx (1)
packages/ui/src/button.tsx (1)
Button(11-20)
apps/web/src/app/[locale]/dashboard/webhooks/page.tsx (1)
apps/web/src/contexts/auth-context.tsx (1)
useAuth(85-91)
🔇 Additional comments (9)
apps/web/src/components/add-webhook-dialog.tsx (2)
5-5: LGTM! Clean i18n integration.The translation setup is properly initialized and the namespace follows a clear naming convention.
Also applies to: 25-25
140-141: Good practice: Documenting non-translated content.The comment correctly identifies that server-side error messages are not translated client-side, which is appropriate since error messages often come from the API and may contain technical details.
apps/web/src/components/avatars/create-avatar-dialog.tsx (1)
5-5: LGTM! Comprehensive translation coverage.All user-facing strings are properly localized, including dynamic content for create/edit modes and loading states.
Also applies to: 44-44
apps/web/src/components/avatars/avatar-card.tsx (1)
5-5: LGTM! Good use of interpolation for dynamic content.The translation implementation is solid, with proper use of ICU message syntax for variable substitution (username and date).
Also applies to: 51-51, 179-181, 205-205
apps/web/src/components/avatars/avatar-selector.tsx (1)
5-5: LGTM! Proper translation implementation.All user-facing strings are properly localized, including conditional empty state messages.
Also applies to: 32-32, 70-70, 81-81, 107-108
apps/web/src/components/webhook-card.tsx (2)
4-4: LGTM! Good use of multiple translation namespaces.Using both component-specific (
webhookCard) and shared (common) namespaces is a good pattern for organizing translations and promoting reusability of common strings.Also applies to: 40-42
240-243: Verify: Placeholder text for incomplete feature.The use of
t('details.todoPlaceholder')for messages sent suggests this is placeholder text for a feature that hasn't been implemented yet. Please confirm whether this is intentional and if there's a plan to implement message counting.If this is a known TODO, consider adding a code comment or tracking it in an issue to ensure it's addressed in a future iteration.
apps/web/src/components/embed-builder.tsx (2)
4-4: LGTM:useTranslationssetup is correct and theembedBuildernamespace exists in apps/web/messages/en.json and es.json.
98-98: All translation keys are defined Verified that every translation key used in embed-builder.tsx exists under embedBuilder in both en.json and es.json.
|
is this PR fine to merge? @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me analyze the PR to identify any remaining issues before it can be merged. 🏁 Script executed: Length of output: 335 🏁 Script executed: Length of output: 2276 🏁 Script executed: Length of output: 615 🏁 Script executed: Length of output: 421 🏁 Script executed: Length of output: 1237 🏁 Script executed: Length of output: 1705 🏁 Script executed: Length of output: 10841 🏁 Script executed: Length of output: 916 🏁 Script executed: Length of output: 2036 🏁 Script executed: Length of output: 1731 🏁 Script executed: Length of output: 1522 @ctrixcode I've completed a thorough analysis of the PR and found several critical issues that need to be fixed before merging: 🚨 Critical Translation Key Mismatches1. Dashboard Empty State Structure MismatchThe code expects flat keys, but the EN translation file uses nested structure: Code usage (webhooks page.tsx): t('empty.noResultsTitle') // Line 201
t('empty.noResultsSubtitle') // Line 204EN translation (incorrect - nested): {
"empty": {
"noResults": {
"title": "No webhooks found",
"desc": "Try adjusting your search query or filters"
}
}
}ES translation (correct - flat): {
"empty": {
"noResultsTitle": "No se encontraron webhooks",
"noResultsSubtitle": "Prueba ajustando tu búsqueda o filtros"
}
}Fix needed: EN should match the ES flat structure. 2. Missing Key in ENThe code uses
Fix needed: Add 3. Extra Unused Keys in ESES has these keys that aren't used in code:
Fix needed: Remove these unused keys from ES file. Other Issues FoundConsole StatementsFound 9 console.log/error statements in production code (mostly for error handling in auth flows). These are generally acceptable for error logging but consider using a proper logging service. Recommendation❌ Not ready to merge yet. The translation key mismatches will cause runtime errors when the app runs with the English locale. Users will see missing translations or errors. Please fix these issues and I can review again:
Would you like me to create an issue to track these fixes, or would you prefer @mdrehan369 to address them in this PR? |
|
@mdrehan369 please fix all the issues |
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: 2
🧹 Nitpick comments (9)
apps/web/src/app/[locale]/dashboard/settings/page.tsx (6)
16-21: Remove the development note comment.The comment on line 20 is a development artifact and should be removed before merging.
import { useTranslations } from 'next-intl'; export default function SettingsPage() { const t = useTranslations('dashboard.settingsPage'); - // NOTE: New translation keys for hardcoded sections are added below (e.g., 'password.title') const tAuth = useTranslations('dashboard.settingsPage.auth');
78-81: Consider a safer pattern for dynamic translation keys.The template literal key construction bypasses TypeScript's type-safety and can fail at runtime if the corresponding translation keys don't exist. The
as constassertion doesn't help here since it's applied to a runtime-constructed string.Consider using an explicit mapping that TypeScript can validate:
const getAccountTypeQuote = (accountType: string) => { - const key = `accountQuotes.${accountType || 'free'}` as const; - return t(key); + const keyMap: Record<string, 'accountQuotes.free' | 'accountQuotes.paid' | 'accountQuotes.premium'> = { + free: 'accountQuotes.free', + paid: 'accountQuotes.paid', + premium: 'accountQuotes.premium', + }; + const key = keyMap[accountType || 'free'] || keyMap.free; + return t(key); };This approach ensures compile-time validation of translation keys and fails fast if a key is missing.
56-63: Consider internationalizing size unit labels.The
formatBytesfunction uses hardcoded English abbreviations for byte sizes. While these abbreviations are commonly used across many locales, some languages may prefer localized labels or full words.If translation resources include size unit labels, consider refactoring to use them:
const formatBytes = (bytes: number, decimals = 2) => { if (bytes === 0) return `0 ${t('units.bytes')}`; const k = 1024; const dm = decimals < 0 ? 0 : decimals; const sizes = ['bytes', 'kb', 'mb', 'gb', 'tb']; const i = Math.floor(Math.log(bytes) / Math.log(k)); return parseFloat((bytes / Math.pow(k, i)).toFixed(dm)) + ' ' + t(`units.${sizes[i]}`); };This can be deferred if not a priority for the current locales.
100-114: Consider translating the account type badge text.The Badge displays the raw
accountTypevalue in uppercase (line 108). For complete internationalization, this should use a translation key as well.<Badge className={ `text-lg px-4 py-2 font-semibold ` + getAccountTypeBadgeClass(user.accountType || 'free') } > - {(user.accountType || 'free').toUpperCase()} + {t(`accountType.${user.accountType || 'free'}`)} </Badge>Ensure the corresponding keys (e.g.,
accountType.free,accountType.paid,accountType.premium) exist in both translation files.
163-173: Remove development artifact comments.The inline
{/* <--- UPDATED */}comments are development artifacts and should be removed before merging.<span className="text-slate-300 font-medium"> - {tAuth('discord.statusConnected')} {/* <--- UPDATED */} + {tAuth('discord.statusConnected')} </span> <span className="text-slate-500 text-sm"> - {tAuth('discord.idLabel')}: {user.discord_id}{' '} - {/* <--- UPDATED */} + {tAuth('discord.idLabel')}: {user.discord_id} </span> <span className="text-slate-500 text-sm"> - {tAuth('discord.usernameLabel')}: {user.username}{' '} - {/* <--- UPDATED */} + {tAuth('discord.usernameLabel')}: {user.username} </span>Similar comments should be removed throughout the file (lines 124-125, 137-138, 147-148, 184).
268-276: Consider localizing the Twitter share text.The Twitter intent URL contains hardcoded English text in the tweet parameter. For a fully internationalized experience, this text should be localized based on the user's locale.
You could construct the URL dynamically:
const shareText = t('support.twitterShareText'); const shareUrl = 'https://webhook.ctrix.pro'; const twitterIntentUrl = `https://twitter.com/intent/tweet?text=${encodeURIComponent(shareText)}&url=${encodeURIComponent(shareUrl)}`;Then use
twitterIntentUrlin the anchor'shref. This would require adding asupport.twitterShareTextkey to your translation files.apps/web/src/components/dashboard-header.tsx (2)
40-99: Simplify badge logic with configuration object.The
getAccountTypeBadgefunction contains repetitive switch cases wherelabelKeyandquoteKeyare always identical. Consider refactoring to a configuration object for better maintainability.Apply this refactor to reduce repetition:
const getAccountTypeBadge = (accountType: string) => { - const commonBadgeClasses = 'ml-2'; - let labelKey: 'free' | 'paid' | 'premium'; - let quoteKey: 'free' | 'paid' | 'premium'; - let badgeContent; - let badgeClasses; - - switch (accountType) { - case 'free': - labelKey = 'free'; - quoteKey = 'free'; - badgeClasses = - 'bg-blue-500/80 text-white text-xs px-1 py-0.5 rounded-full'; - badgeContent = t(`accountType.${labelKey}`); - break; - case 'paid': - labelKey = 'paid'; - quoteKey = 'paid'; - badgeClasses = - 'bg-purple-500/80 text-white text-xs px-1 py-0.5 rounded-full'; - badgeContent = t(`accountType.${labelKey}`); - break; - case 'premium': - labelKey = 'premium'; - quoteKey = 'premium'; - badgeClasses = 'h-4 w-4 text-yellow-400 drop-shadow-md'; - badgeContent = ( - <Crown - className={`${commonBadgeClasses} ${badgeClasses}`} - fill="currentColor" - /> - ); - break; - default: - return null; - } + const config = { + free: { + classes: 'bg-blue-500/80 text-white text-xs px-1 py-0.5 rounded-full', + content: t('accountType.free'), + }, + paid: { + classes: 'bg-purple-500/80 text-white text-xs px-1 py-0.5 rounded-full', + content: t('accountType.paid'), + }, + premium: { + classes: 'h-4 w-4 text-yellow-400 drop-shadow-md', + content: <Crown className="ml-2 h-4 w-4 text-yellow-400 drop-shadow-md" fill="currentColor" />, + }, + } as const; + + const badge = config[accountType as keyof typeof config]; + if (!badge) return null; + + const key = accountType as 'free' | 'paid' | 'premium'; + const tooltipQuote = t(`accountTypeTooltip.${key}`); - const tooltipQuote = t(`accountTypeTooltip.${quoteKey}`); - return ( <TooltipProvider> <Tooltip> <TooltipTrigger asChild> - {accountType === 'premium' ? ( - // Crown is rendered directly for premium - badgeContent + {key === 'premium' ? ( + badge.content ) : ( - // Badge is rendered for free/paid - <Badge className={`${commonBadgeClasses} ${badgeClasses}`}> - {badgeContent} + <Badge className={`ml-2 ${badge.classes}`}> + {badge.content} </Badge> )} </TooltipTrigger> <TooltipContent className="bg-slate-800 border-slate-700 text-white"> <p>{tooltipQuote}</p> </TooltipContent> </Tooltip> </TooltipProvider> ); };
124-124: Remove verbose inline comments.The inline comments explaining each translation call (lines 124, 126, 138, 155, 191, 200) are unnecessary as the
t()function calls are self-documenting. Reserve comments for non-obvious logic.Apply this diff to clean up the comments:
- {/* Translate Title */} <h1 className="text-lg font-semibold text-white">{t('title')}</h1> - {/* Translate Subtitle */} <p className="text-xs text-slate-400">{t('subtitle')}</p>- {/* Translate "Lang" and display current locale */} {t('languageSelector')} ({locale})- {/* Assuming localeMap contains the translated language name (e.g., 'en' -> 'English') */} {localeMap[loc as keyof typeof localeMap]}<Settings className="mr-2 h-4 w-4" /> - {/* Translate Settings menu item */} <span>{t('menu.settings')}</span><LogOut className="mr-2 h-4 w-4" /> - {/* Translate Log out menu item */} <span>{t('menu.logout')}</span>Also applies to: 126-126, 138-138, 155-155, 191-191, 200-200
apps/web/messages/es.json (1)
112-117: Remove unusedfilters.searchPlaceholdertranslation key.Verification confirms
filters.searchPlaceholder(line 116) is unused in the codebase. The search input inapps/web/src/app/[locale]/dashboard/webhooks/page.tsx:121usessearch.placeholderinstead. Remove the duplicate unused key to maintain translation file cleanliness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/web/messages/en.json(1 hunks)apps/web/messages/es.json(1 hunks)apps/web/src/app/[locale]/dashboard/settings/page.tsx(8 hunks)apps/web/src/app/[locale]/login/page.tsx(11 hunks)apps/web/src/components/avatars/create-avatar-dialog.tsx(12 hunks)apps/web/src/components/dashboard-header.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/components/avatars/create-avatar-dialog.tsx
- apps/web/src/app/[locale]/login/page.tsx
- apps/web/messages/en.json
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/app/[locale]/dashboard/settings/page.tsx (2)
apps/web/src/components/settings/settings-card.tsx (1)
SettingsCard(20-53)apps/web/src/components/ui/confirmation-dialog.tsx (1)
ConfirmationDialog(23-55)
apps/web/src/components/dashboard-header.tsx (3)
apps/web/src/contexts/auth-context.tsx (1)
useAuth(85-91)packages/ui/src/button.tsx (1)
Button(11-20)apps/web/src/i18n/routing.ts (1)
localeMap(3-6)
🔇 Additional comments (1)
apps/web/src/components/dashboard-header.tsx (1)
132-160: LGTM! Language selector implementation is correct.The language selector properly uses the i18n navigation router with locale switching via
router.replace(pathname, { locale }), which is the recommended next-intl pattern. The implementation correctly iterates overlocaleMapand displays translated language names.
| "empty": { | ||
| "noResultsTitle": "No se encontraron webhooks", | ||
| "noResultsSubtitle": "Prueba ajustando tu búsqueda o filtros", | ||
| "noWebhooks": { | ||
| "title": "Aún no hay webhooks", | ||
| "desc": "Comienza agregando tu primer webhook de Discord" | ||
| }, | ||
| "noWebhooksSubtitle": "Comienza agregando tu primer webhook de Discord" | ||
| }, |
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.
🧩 Analysis chain
Remove duplicate key and verify empty state structure consistency.
The noWebhooksSubtitle key at line 125 appears to be a duplicate of the nested noWebhooks.desc structure and was flagged as unused in previous bot reviews. Additionally, the mixed nesting pattern here (flat keys + nested object) may cause inconsistencies with the code's expectations.
Apply this diff to remove the duplicate key:
"empty": {
"noResultsTitle": "No se encontraron webhooks",
"noResultsSubtitle": "Prueba ajustando tu búsqueda o filtros",
"noWebhooks": {
"title": "Aún no hay webhooks",
"desc": "Comienza agregando tu primer webhook de Discord"
- },
- "noWebhooksSubtitle": "Comienza agregando tu primer webhook de Discord"
+ }
},Verify that the code consuming these keys expects the nested noWebhooks.title and noWebhooks.desc structure rather than flat keys:
🏁 Script executed:
#!/bin/bash
# Search for usage of dashboard.empty translation keys in the codebase
rg -nP --type=tsx --type=ts -C3 "t\(['\"]empty\."Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for usage of empty state translation keys without problematic file types
echo "=== Searching for empty key usage ==="
rg -nP -C3 "empty\." --type=ts
echo -e "\n=== Searching specifically for noWebhooksSubtitle ==="
rg -nP "noWebhooksSubtitle" -C3
echo -e "\n=== Searching for noWebhooks usage (nested structure) ==="
rg -nP "noWebhooks\." -C3 --type=ts
echo -e "\n=== Looking for common translation usage patterns ==="
rg -nP 'translate|t\(' apps/web --type=js --type=ts -C2 | head -40Length of output: 7180
Remove unused noWebhooksSubtitle key from empty state structure.
The noWebhooksSubtitle key at line 125 is not referenced anywhere in the codebase. The webhooks page uses the nested empty.noWebhooks.title and empty.noWebhooks.desc structure (lines 211–214 of apps/web/src/app/[locale]/dashboard/webhooks/page.tsx), making the flat key redundant.
Apply this diff:
"empty": {
"noResultsTitle": "No se encontraron webhooks",
"noResultsSubtitle": "Prueba ajustando tu búsqueda o filtros",
"noWebhooks": {
"title": "Aún no hay webhooks",
"desc": "Comienza agregando tu primer webhook de Discord"
- },
- "noWebhooksSubtitle": "Comienza agregando tu primer webhook de Discord"
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "empty": { | |
| "noResultsTitle": "No se encontraron webhooks", | |
| "noResultsSubtitle": "Prueba ajustando tu búsqueda o filtros", | |
| "noWebhooks": { | |
| "title": "Aún no hay webhooks", | |
| "desc": "Comienza agregando tu primer webhook de Discord" | |
| }, | |
| "noWebhooksSubtitle": "Comienza agregando tu primer webhook de Discord" | |
| }, | |
| "empty": { | |
| "noResultsTitle": "No se encontraron webhooks", | |
| "noResultsSubtitle": "Prueba ajustando tu búsqueda o filtros", | |
| "noWebhooks": { | |
| "title": "Aún no hay webhooks", | |
| "desc": "Comienza agregando tu primer webhook de Discord" | |
| } | |
| }, |
🤖 Prompt for AI Agents
In apps/web/messages/es.json around lines 118 to 126, remove the unused
"noWebhooksSubtitle" key (currently at line 125) from the "empty" object; delete
that key and the trailing comma/spacing as needed so the JSON remains valid,
leaving only the nested "noWebhooks" object with "title" and "desc".
| "namePlaceholder": "Mi Plantilla Genial", | ||
| "descriptionLabel": "Descripción (Opcional)", | ||
| "descriptionPlaceholder": "¿Para qué sirve esta plantilla? Describe su propósito...", | ||
| "livePreviewHeader": "Vista Previa en Vivo" |
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.
🧩 Analysis chain
Verify usage of livePreviewHeader key.
The livePreviewHeader key was flagged in previous reviews as potentially unused. Confirm whether this key is referenced in the template form component.
If the key is unused, remove it to keep the translation file clean.
🏁 Script executed:
#!/bin/bash
# Search for usage of livePreviewHeader translation key
rg -nP --type=tsx --type=ts -C3 "t\(['\"].*livePreviewHeader"Length of output: 27
🏁 Script executed:
#!/bin/bash
# Retry search for livePreviewHeader usage without specific file type
# First, search for any usage of this key
rg -nC3 "livePreviewHeader" --type=js --type=tsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Broader search without type restrictions
echo "=== Searching for livePreviewHeader in all files ==="
rg -n "livePreviewHeader"
echo "=== Searching for livePreview patterns ==="
rg -n "livePreview" --max-count=20
echo "=== Checking for dynamic references in es.json ==="
fd . --name "*.json" --exec grep -l "livePreviewHeader" {} \;Length of output: 539
Remove unused livePreviewHeader translation key from es.json.
The livePreviewHeader key at line 480 is not referenced anywhere in the codebase. It appears only in the translation file and should be removed to keep translations synchronized with actual usage.
🤖 Prompt for AI Agents
In apps/web/messages/es.json at line 480, remove the unused translation key
"livePreviewHeader": "Vista Previa en Vivo" because it's not referenced anywhere
in the codebase; update the JSON by deleting that key (and any trailing comma
adjustments) to keep translations synchronized with actual usage.
Issue #8
Summary by CodeRabbit
New Features
Chores