-
Notifications
You must be signed in to change notification settings - Fork 0
Fix auth & made lang switch available when logged out #21
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
Conversation
WalkthroughThis update restructures internationalization and authentication. The translation file now separates language and theme options from the account settings. In the authentication module, user emails become dynamically generated using Discord IDs, and a new middleware hook sets the locale after authentication. Additionally, the Navbar import path is updated, and several navigation components are refactored—removing in-component locale management in favor of modular Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant A as Auth Service
participant M as Auth Middleware
U->>A: Initiate login
A->>M: Call after hook on "/callback"
M->>A: Retrieve session
M->>A: Set user locale
A-->>U: Return authenticated session
sequenceDiagram
participant U as User
participant ND as NavDropdown
participant LS as LanguageSwitcher
participant TS as ThemeSwitcher
U->>ND: Click dropdown button
ND->>LS: Display language options
LS->>LS: Update locale (trigger updateUser)
ND->>TS: Display theme options
TS->>TS: Change theme (trigger setTheme)
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/auth.ts (1)
46-53: Added middleware hook to set user locale after authentication.The middleware correctly sets the user's locale after the authentication process, but only for callback paths which might not cover all cases where locale needs to be updated.
Consider adding a comment explaining why this is only done for callback paths, or implementing a more comprehensive solution that handles locale setting in other contexts as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/discord-login.tsx(2 hunks)src/components/language-switcher.tsx(1 hunks)src/components/navbar.tsx(4 hunks)src/i18n/translations/en.json(1 hunks)src/lib/auth.ts(3 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
src/components/discord-login.tsx (1)
src/components/language-switcher.tsx (1)
LanguageSwitcher(18-92)
src/lib/auth.ts (1)
src/i18n/services/locale.ts (1)
setUserLocale(12-14)
src/components/navbar.tsx (2)
src/components/ui/dropdown-menu.tsx (5)
DropdownMenu(258-258)DropdownMenuTrigger(260-260)DropdownMenuContent(261-261)DropdownMenuGroup(262-262)DropdownMenuItem(264-264)src/components/language-switcher.tsx (1)
LanguageSwitcher(18-92)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Format Files
- GitHub Check: Build Application
🔇 Additional comments (11)
src/i18n/translations/en.json (1)
169-174: Well-structured translation keys for language and theme!The new dropdown section with language and theme translations nicely separates these options from the account settings, making them more accessible.
src/components/discord-login.tsx (1)
3-4: Imports updated correctly for new language switcher component.The necessary imports have been added to support the LanguageSwitcher component.
Also applies to: 8-8
src/lib/auth.ts (2)
2-2: Added required imports for locale handling.The imports for setUserLocale and createAuthMiddleware are correctly added to support the new locale functionality.
Also applies to: 6-6
33-33: Improved email generation using Discord ID.Using the Discord ID to generate a unique email address is a good practice as it ensures unique identification for each user.
src/components/language-switcher.tsx (3)
18-22: Good initialization of LanguageSwitcher component.The component is properly set up with the necessary hooks and state initialization.
31-40: Locale change handler looks good.The handleLocaleChange function correctly updates both the local state and user preferences. It also has a proper conditional check before updating the user profile.
42-91: Well-structured dropdown menu for language selection.The language selection dropdown is well-organized with proper flag icons and language names. The implementation uses the appropriate UI components for the dropdown menu.
src/components/navbar.tsx (4)
4-4: Import statements updated to support new dropdown functionality.The imports have been updated to include the necessary components for the new navigation dropdown menu. Icons like
Menu,Sun, andMoonare added for UI elements, and the newLanguageSwitchercomponent is imported to handle language selection functionality. The dropdown menu components are also properly imported from the UI library.Also applies to: 11-18
83-83: Replaced ThemeToggle with NavDropdown component in mobile and desktop views.The ThemeToggle component has been replaced with NavDropdown in both the mobile view (line 83) and desktop view (line 90). This change correctly implements the PR objective to make language switching available when logged out by placing it in the navigation bar.
Also applies to: 90-90
164-168: Function renamed from ThemeToggle to NavDropdown with updated translation hook.The function has been renamed to better reflect its expanded functionality, now including both theme and language options. The translation hook is now used without a specific namespace, allowing access to nested translation keys used in the dropdown menu.
177-203: Implemented dropdown menu with theme toggle and language switcher functionality.The implementation now uses a dropdown menu structure that elegantly combines theme toggling and language switching in a single UI element. The dropdown is triggered by a menu icon and includes:
- Theme toggle functionality that maintains the original behavior while adding localized text
- The new LanguageSwitcher component
This implementation effectively addresses the PR objective by making language switching available in the navigation bar, accessible to all users including those who are logged out.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
🧹 Nitpick comments (4)
src/components/language-switcher.tsx (4)
79-79: Language name correction needed.The Spanish language should be written as "Español" (the noun) rather than "Española" (feminine adjective).
- Española + Español
86-86: Language name correction needed.The Italian language should be written as "Italiano" (the noun) rather than "Italiana" (feminine adjective).
- Italiana + Italiano
23-29: Consider adding error handling for locale fetching.The useEffect hook doesn't include error handling if
getUserLocale()fails. Consider adding a try/catch block to handle potential errors gracefully.useEffect(() => { async function getLang() { + try { setLang(await getUserLocale()); + } catch (error) { + console.error("Failed to get user locale:", error); + // Fallback to default language + setLang("en"); + } } getLang(); }, []);
21-21: Consider adding a loading state.Adding a loading state while fetching the user locale would improve the user experience, especially on slower connections.
- const [lang, setLang] = useState("en"); + const [lang, setLang] = useState("en"); + const [isLoading, setIsLoading] = useState(true); useEffect(() => { async function getLang() { try { + setIsLoading(true); setLang(await getUserLocale()); } catch (error) { console.error("Failed to get user locale:", error); // Fallback to default language setLang("en"); } finally { + setIsLoading(false); } } getLang(); }, []);Then you could use the loading state in your render method to display a loading indicator if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/language-switcher.tsx(1 hunks)
🔇 Additional comments (2)
src/components/language-switcher.tsx (2)
23-29: Good implementation of the useEffect dependency array.I see you've properly implemented the empty dependency array in the useEffect hook, ensuring it only runs once when the component mounts. This fixes the performance issue noted in previous reviews.
31-40: Well-structured language change handler.The
handleLocaleChangefunction properly handles both UI state and user profile updates when a language is selected. The conditional check for an active session ensures that user updates are only attempted when appropriate.
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
🧹 Nitpick comments (2)
src/components/nav/theme-switcher.tsx (1)
16-38: Clean and accessible UI implementationThe dropdown menu structure is well-implemented with appropriate icons for visual feedback. The radio group correctly uses the theme value and handles changes properly.
One suggestion to enhance this implementation:
<DropdownMenuRadioGroup value={theme} - onValueChange={(value) => setTheme(value)} + onValueChange={setTheme} >This is a minor optimization as
setThemecan be passed directly as the handler since it accepts the same argument that onValueChange provides.src/components/nav/language-switcher.tsx (1)
42-91: Well-structured language selection UIThe dropdown implementation is clean and provides a good user experience with flag icons for each language. All common languages are represented.
One minor issue to consider:
The Spanish language name should be "Español" rather than "Española" (which refers to a Spanish woman rather than the language).
<DropdownMenuRadioItem value="es"> <Flag code="es" className="size-4" /> - Española + Español </DropdownMenuRadioItem>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/layout.tsx(1 hunks)src/components/nav/discord-login.tsx(4 hunks)src/components/nav/language-switcher.tsx(1 hunks)src/components/nav/navbar.tsx(3 hunks)src/components/nav/theme-switcher.tsx(1 hunks)src/i18n/translations/en.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/i18n/translations/en.json
🧰 Additional context used
🧬 Code Definitions (2)
src/components/nav/navbar.tsx (2)
src/components/nav/theme-switcher.tsx (1)
ThemeSwitcher(12-40)src/components/nav/language-switcher.tsx (1)
LanguageSwitcher(18-92)
src/components/nav/discord-login.tsx (2)
src/components/nav/theme-switcher.tsx (1)
ThemeSwitcher(12-40)src/components/nav/language-switcher.tsx (1)
LanguageSwitcher(18-92)
🔇 Additional comments (15)
src/components/nav/discord-login.tsx (4)
3-4: Import consolidation looks goodThe updated imports for Lucide icons and next-intl are appropriate for the component's needs.
7-8: Import path updates are correctThe component import paths have been properly updated to use relative paths, which is a good practice for components in the same directory.
22-23: Good modular approach with new component importsThe imports for the new
LanguageSwitcherandThemeSwitchercomponents indicate a nice separation of concerns, moving functionality to dedicated components.
72-73: Well implemented component integrationThe
ThemeSwitcherandLanguageSwitchercomponents are properly integrated into the dropdown menu, replacing the previous in-component implementation. This makes the code more maintainable and follows the single responsibility principle.src/components/nav/theme-switcher.tsx (3)
1-7: Import structure is well organizedThe dropdown menu component imports are properly structured and specific to the needs of this component.
8-10: Appropriate hooks and icon importsThe necessary Lucide icons for theme representation and the required hooks for theme management and translations are correctly imported.
12-15: Well structured component initializationThe component properly uses the useTranslations hook with the correct namespace and correctly extracts theme information from the useTheme hook.
src/components/nav/navbar.tsx (5)
3-20: Appropriate imports for enhanced functionalityThe imports have been updated to include authentication client, necessary UI components, and the new theme and language switchers. This sets up the component properly for the new functionality.
25-25: Good session management implementationAdding session state retrieval from authClient enables the conditional rendering of UI elements based on authentication status.
86-86: Proper conditional rendering for mobile viewThe component correctly handles showing the NavDropdown only when user is not logged in or authentication is not pending in the mobile menu.
93-93: Consistent conditional rendering for desktop viewThe same conditional logic is correctly applied to the desktop view, maintaining consistency across breakpoints.
167-189: Well-implemented dropdown for user preferencesThe NavDropdown function is clean, well-structured, and properly integrates both theme and language switchers. The Button component is correctly used as a trigger with appropriate styling.
src/components/nav/language-switcher.tsx (3)
1-17: Appropriate imports and setupThe necessary imports for locale management, UI components, and internationalization are correctly included.
18-30: Effective state management and initializationThe component correctly uses useState for local state management and useEffect to initialize the language setting. The async function for retrieving the user locale is properly implemented.
31-40: Proper locale change handlingThe locale change handler correctly updates both the local UI state and the user's saved preference. It properly checks for an active session before attempting to update user settings.
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(1 hunks)src/components/nav/discord-login.tsx(5 hunks)src/components/nav/theme-switcher.tsx(1 hunks)src/components/ui/dropdown-menu.tsx(1 hunks)src/i18n/translations/en.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/components/ui/dropdown-menu.tsx
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/i18n/translations/en.json
🧰 Additional context used
🧬 Code Definitions (1)
src/components/nav/discord-login.tsx (2)
src/components/nav/theme-switcher.tsx (1)
ThemeSwitcher(12-45)src/components/nav/language-switcher.tsx (1)
LanguageSwitcher(18-92)
🔇 Additional comments (6)
src/components/nav/discord-login.tsx (3)
1-3: Good modular component integrationThe additions of
LanguageSwitcherandThemeSwitcherimports reflect a positive shift toward modular component architecture. This approach improves maintainability by separating concerns into dedicated components.
47-62: Enhanced UI with structured button triggerReplacing the simple dropdown trigger with a fully structured Button component improves usability and accessibility. The avatar integration with proper spacing and text layout creates a more polished user experience.
80-81: LGTM - Functionality matches PR objectivesThe integration of
ThemeSwitcherandLanguageSwitchercomponents directly addresses the PR objective of making language switching available when logged out, while maintaining a consistent UX pattern for both authenticated and unauthenticated users.src/components/nav/theme-switcher.tsx (3)
1-10: Clean imports with good component organizationThe imports are well-organized, providing all necessary UI components, icons, and hooks for the theme switcher functionality.
12-15: Well-structured component with proper hook usageGood use of the
useTranslationshook with a specific namespace ("nav.dropdown.theme") and theuseThemehook for managing theme state. This ensures that the component is properly integrated with the application's i18n and theming systems.
16-44: Well-implemented theme switching UI with accessible optionsThe dropdown implementation provides a clean, accessible UI for theme switching. Each option includes both an icon and translated text, enhancing usability. The radio group pattern correctly maintains the current selection state.
Fixes the auth issues and allows the language to be switched while you are logged out through a new menu button on the nav bar.
Logged out
Logged In
Summary by CodeRabbit
New Features
Bug Fixes