[WEB-5582] chore: top nav and user menu improvement#8245
[WEB-5582] chore: top nav and user menu improvement#8245sriramveeraghanta merged 2 commits intopreviewfrom
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughTwo UI components were updated: styling adjustments to flex containers and input margins in a navigation customization dialog, and refactoring of navigation items in a user menu from Link components to CustomMenu.MenuItem with imperative router.push() calls for navigation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors navigation patterns in the user menu and improves radio button alignment in the navigation preferences dialog. The changes replace Next.js Link components with router.push() callbacks in menu items, aligning with the established pattern used throughout sidebar components in the codebase.
Key changes:
- Migrated from wrapping
CustomMenu.MenuItemwithLinkto usingonClickhandlers withrouter.push() - Fixed radio button alignment by adjusting flex properties and adding top margin
- Removed unused
Linkimport and addeduseRouterfrom "next/navigation"
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/web/core/components/workspace/sidebar/user-menu-root.tsx | Refactored navigation from Link wrapping to onClick with router.push for Settings, Preferences, and God Mode menu items |
| apps/web/core/components/navigation/customize-navigation-dialog.tsx | Fixed radio button vertical alignment by removing items-center and adding mt-1 to input elements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/core/components/workspace/sidebar/user-menu-root.tsx (1)
95-104: Consider aligning Sign Out with the new navigation pattern.The Sign Out item uses a
buttoninsideCustomMenu.MenuItem, while the newly refactored Settings, Preferences, and God Mode items useCustomMenu.MenuItemwithonClickdirectly. For consistency, consider refactoring Sign Out to match the new pattern.Apply this diff to align the pattern:
- <div className={`${isUserInstanceAdmin ? "pb-2" : ""}`}> - <CustomMenu.MenuItem> - <button - type="button" - className="flex w-full items-center gap-2 rounded text-xs hover:bg-custom-background-80" - onClick={handleSignOut} - > - <LogOut className="size-4 stroke-[1.5]" /> - {t("sign_out")} - </button> - </CustomMenu.MenuItem> - </div> + <div className={`${isUserInstanceAdmin ? "pb-2" : ""}`}> + <CustomMenu.MenuItem onClick={handleSignOut}> + <div className="flex w-full items-center gap-2 rounded text-xs"> + <LogOut className="size-4 stroke-[1.5]" /> + {t("sign_out")} + </div> + </CustomMenu.MenuItem> + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/navigation/customize-navigation-dialog.tsx(2 hunks)apps/web/core/components/workspace/sidebar/user-menu-root.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/navigation/customize-navigation-dialog.tsxapps/web/core/components/workspace/sidebar/user-menu-root.tsx
🧠 Learnings (2)
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/workspace/sidebar/user-menu-root.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/workspace/sidebar/user-menu-root.tsx
🧬 Code graph analysis (1)
apps/web/core/components/workspace/sidebar/user-menu-root.tsx (4)
packages/ui/src/dropdowns/custom-menu.tsx (1)
CustomMenu(532-532)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)packages/i18n/src/store/index.ts (1)
t(223-244)packages/constants/src/endpoints.ts (1)
GOD_MODE_URL(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
apps/web/core/components/navigation/customize-navigation-dialog.tsx (1)
262-294: LGTM! Improved radio button alignment pattern.The change from
flex items-centertoflex gap-2withmt-1on the input aligns the radio button with the first line of text rather than centering it vertically with the entire text block. This is a better UX pattern when labels include multi-line descriptions.apps/web/core/components/workspace/sidebar/user-menu-root.tsx (2)
3-3: LGTM! Correct router setup for App Router.The import and initialization of
useRouterfromnext/navigationis appropriate for imperative navigation in menu items.Also applies to: 24-25
110-114: LGTM! God Mode navigation follows the new pattern.The refactor to
CustomMenu.MenuItemwithrouter.push(GOD_MODE_URL)is consistent with the Settings and Preferences changes, and properly uses translation.
| </div> | ||
| </CustomMenu.MenuItem> | ||
| </Link> | ||
| <CustomMenu.MenuItem onClick={() => router.push(`/${workspaceSlug}/settings/account`)}> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, read the component file to see the useParams() usage
cat -n apps/web/core/components/workspace/sidebar/user-menu-root.tsx | head -100Repository: makeplane/plane
Length of output: 4286
🏁 Script executed:
# Find where UserMenuRoot is imported and used
rg -n "UserMenuRoot" --type tsx --type ts -C 2Repository: makeplane/plane
Length of output: 86
🏁 Script executed:
# Check the useParams hook and its type signature in the codebase
rg -n "useParams" apps/web/core/components/workspace/sidebar/user-menu-root.tsx -A 3 -B 3Repository: makeplane/plane
Length of output: 529
🏁 Script executed:
# Search for UserMenuRoot imports and usage
rg "UserMenuRoot" --type ts -C 3Repository: makeplane/plane
Length of output: 2208
🏁 Script executed:
# Check the actual routes where UserMenuRoot might be used
rg "sidebar" apps/web/core/components/workspace --type ts -l | head -10Repository: makeplane/plane
Length of output: 719
🏁 Script executed:
# Look for type definitions or custom hooks for useParams
fd "use-params" apps/webRepository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Check if there's a Next.js configuration that might affect useParams
cat tsconfig.json | head -50Repository: makeplane/plane
Length of output: 104
🏁 Script executed:
# Check the top-navigation-root.tsx file where UserMenuRoot is used
cat -n apps/web/ce/components/navigations/top-navigation-root.tsx | head -50Repository: makeplane/plane
Length of output: 2461
🏁 Script executed:
# Look for app router structure with workspaceSlug
find apps/web -type f -name "layout.tsx" -o -name "page.tsx" | grep -E "workspace|workspaceSlug" | head -20Repository: makeplane/plane
Length of output: 1631
🏁 Script executed:
# Search for route structure with [workspaceSlug]
rg "\[workspaceSlug\]" --type ts -lRepository: makeplane/plane
Length of output: 1003
🏁 Script executed:
# Check the actual route segments in the app directory
find apps/web -type d -name "\[workspaceSlug\]" 2>/dev/nullRepository: makeplane/plane
Length of output: 93
🏁 Script executed:
# Find tsconfig.json in the workspace root or apps/web
find apps/web -name "tsconfig.json" -o -name "tsconfig*.json" | head -5Repository: makeplane/plane
Length of output: 81
🏁 Script executed:
# Check if there's a root tsconfig
find . -maxdepth 2 -name "tsconfig.json" 2>/dev/nullRepository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Look at how other components in the same file handle useParams
rg "useParams()" apps/web/core/components/workspace/sidebar --type ts -A 3 -B 1Repository: makeplane/plane
Length of output: 16068
🏁 Script executed:
# Check if workspaceSlug is used elsewhere without guards in same directory
rg "workspaceSlug" apps/web/core/components/workspace/sidebar/user-menu-root.tsx -C 5Repository: makeplane/plane
Length of output: 1232
🏁 Script executed:
# Check TypeScript strictNullChecks setting
cat apps/web/tsconfig.json | grep -A 5 "strictNullChecks\|compilerOptions"Repository: makeplane/plane
Length of output: 388
Add type guards for workspaceSlug and translate the "Preferences" label.
workspaceSlug from useParams() is potentially undefined under strict null checks (strictNullChecks: true in tsconfig). This would result in navigating to /${undefined}/settings/account. Add a guard or assertion (see quick-actions.tsx for the pattern in this codebase). Additionally, the "Preferences" label on line 89 is hardcoded while "settings" on line 83 is translated—apply the same t() call for consistency.
🤖 Prompt for AI Agents
In apps/web/core/components/workspace/sidebar/user-menu-root.tsx around line 80,
add a type guard/assertion for workspaceSlug (from useParams()) before using it
in router.push to avoid navigating to /undefined — follow the pattern used in
quick-actions.tsx (check for null/undefined and return early or assert with a
non-null operator where appropriate). Also replace the hardcoded "Preferences"
label on line 89 with the translated string via t() to match the translated
"settings" label usage.
| <CustomMenu.MenuItem onClick={() => router.push(`/${workspaceSlug}/settings/account/preferences`)}> | ||
| <div className="flex w-full items-center gap-2 rounded text-xs"> | ||
| <Settings2 className="h-4 w-4 stroke-[1.5]" /> | ||
| <span>Preferences</span> |
There was a problem hiding this comment.
Use translation for "Preferences" label.
Line 89 hardcodes "Preferences" while line 83 uses {t("settings")} for consistency with i18n. This prevents proper localization.
Apply this diff to use translation:
- <span>Preferences</span>
+ <span>{t("preferences")}</span>📝 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.
| <span>Preferences</span> | |
| <span>{t("preferences")}</span> |
🤖 Prompt for AI Agents
In apps/web/core/components/workspace/sidebar/user-menu-root.tsx around line 89,
the "Preferences" label is hardcoded; replace the literal string with the i18n
call (e.g., use {t("preferences")}) to match the pattern used for
{t("settings")} for proper localization, and add the "preferences" key to the
locale translation files if it does not already exist.
* chore: user menu code refactor * chore: CustomizeNavigationDialog enhancements
* chore: user menu code refactor * chore: CustomizeNavigationDialog enhancements
* chore: user menu code refactor * chore: CustomizeNavigationDialog enhancements
Description
This PR includes top nav and user menu improvements.
Type of Change
Summary by CodeRabbit
Style
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.