[WEB-5511] regression: revamped navigation UI bugs #8183
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdjusts layout/flex rules across several navigation and sidebar components, consolidates navigation-item derivation in the customize dialog, introduces a SetAsDefaultIcon (and its export), adds support for a "stickies" sidebar item, and tweaks preference update/error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant U as UI (Customize Dialog)
participant P as Preferences API
participant T as Toasts
Note over U,P: User toggles "set default" on a tab
U->>P: updatePreferences({ default_tab: key })
alt success
P-->>U: resolved
U->>T: showSuccess("Default tab updated")
else failure
P-->>U: rejected
U->>T: showError("Failed to update default")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,mts,cts}📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
Files:
🧬 Code graph analysis (1)apps/web/core/components/navigation/use-tab-preferences.ts (3)
⏰ 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). (3)
🔇 Additional comments (1)
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 addresses UI bugs in the revamped navigation system, focusing on styling improvements, better error handling, and icon consistency across navigation components.
Key Changes
- Added new
SetAsDefaultIconcomponent to replace the generic star icon for setting default tabs - Improved layout and styling consistency across navigation components with better Tailwind class usage
- Enhanced user feedback by adding toast notifications for default tab updates
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/propel/src/icons/set-as-default-icon.tsx | New icon component for the "set as default" action |
| packages/propel/src/icons/index.ts | Exported the new SetAsDefaultIcon component |
| apps/web/core/components/workspace/sidebar/workspace-menu-root.tsx | Simplified useEffect logic and improved layout classes with w-fit and whitespace-nowrap |
| apps/web/core/components/navigation/use-tab-preferences.ts | Added toast notifications for successful and failed default tab updates |
| apps/web/core/components/navigation/top-nav-power-k.tsx | Improved styling consistency, simplified class conditionals, and added role="button" for accessibility |
| apps/web/core/components/navigation/tab-navigation-visible-item.tsx | Updated context menu to use SetAsDefaultIcon instead of text-only display |
| apps/web/core/components/navigation/tab-navigation-root.tsx | Improved flex layout with better shrink-0 usage and wrapped ProjectActionsMenu in a non-shrinking container |
| apps/web/core/components/navigation/tab-navigation-overflow-menu.tsx | Replaced Star icon with SetAsDefaultIcon, improved hover states with group modifiers, and added tooltips |
| apps/web/core/components/navigation/project-header.tsx | Removed unused cn import and simplified wrapper div classes |
| apps/web/core/components/navigation/project-actions-menu.tsx | Reorganized variable declarations with comment grouping and renamed type to Props |
| apps/web/core/components/navigation/customize-navigation-dialog.tsx | Simplified UI by removing separated enabled/disabled and pinned/unpinned sections, showing all items in a single sortable list with correct checkbox states from preferences |
| apps/web/ce/components/workspace/sidebar/helper.tsx | Added icon support for stickies navigation item |
| apps/web/ce/components/navigations/top-navigation-root.tsx | Improved flexbox layout with flex-1 on sides and shrink-0 in center for better centering of search component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/web/core/components/navigation/tab-navigation-overflow-menu.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (1)
apps/web/core/components/navigation/customize-navigation-dialog.tsx (1)
133-147: DerivedisEnabledproperty is computed but not used in the Checkbox.The
personalItemsuseMemo computesisEnabledfor each item (line 136), but the Checkbox at line 209 accessespersonalPreferences.items[item.key]?.enableddirectly instead of usingitem.isEnabled. This inconsistency could lead to subtle bugs if the derivation logic differs.Consider using the derived property for consistency:
<Checkbox - checked={!!personalPreferences.items[item.key]?.enabled} + checked={item.isEnabled} onChange={(e) => togglePersonalItem(item.key, e.target.checked)} />
🧹 Nitpick comments (8)
apps/web/ce/components/workspace/sidebar/helper.tsx (1)
35-36: Stickies sidebar icon wiring is correct and consistentThe
"stickies"case returnsMultipleStickyIconwith the samesize-4 flex-shrink-0classes as other items, so layout and visual consistency are preserved. As a future improvement, you could narrowkeyfromstringto a union of supported keys (including"stickies") for better type safety.apps/web/core/components/workspace/sidebar/workspace-menu-root.tsx (1)
74-74: Optional: Remove redundantwhitespace-nowrapclass.The
truncateutility already applieswhite-space: nowrap, making the explicitwhitespace-nowrapredundant. While this doesn't cause issues, removing it would clean up the className.Apply this diff to remove the redundancy:
- className={cn("relative h-full flex max-w-48 w-fit whitespace-nowrap truncate", { + className={cn("relative h-full flex max-w-48 w-fit truncate", {apps/web/core/components/navigation/customize-navigation-dialog.tsx (1)
239-242: Consider using the derivedisPinnedproperty for consistency.The
workspaceItemsarray already computesisPinnedat line 86. Usingitem.isPinnedinstead of re-accessing the preferences would be more consistent with the data model and avoid redundant lookups.<Checkbox - checked={!!workspacePreferences.items[item.key]?.is_pinned} + checked={item.isPinned} onChange={(e) => handleWorkspaceItemToggle(item.key, e.target.checked)} />apps/web/core/components/navigation/top-nav-power-k.tsx (1)
210-225: New trigger layout and focus wiring are solid; consider tighteningrole="button"semanticsThe relative container + width transition and border handling make the Cmd‑K input feel polished, and registering
inputRefwithsetTopNavInputRefis a nice touch for keyboard shortcuts. Since the wrapperdivnow hasrole="button", you might optionally addtabIndex={0}and key handlers for Enter/Space, or drop the role and rely on the actual<input>for accessibility if the wrapper itself doesn’t need to be an interactive control.apps/web/core/components/navigation/tab-navigation-root.tsx (1)
169-187: Header/rail layout refactor should help with overflow; just sanity‑check narrow widthsSeparating
ProjectHeaderandProjectActionsMenuinto ashrink-0group and tightening the divider (shrink-0 h-5 w-1) makes the top rail more predictable and should avoid the actions button being clipped. It’s worth verifying on very narrow viewports that the combinedshrink-0header block plus tabs still behave as expected given theoverflow-hiddenouter container.apps/web/core/components/navigation/project-actions-menu.tsx (1)
33-40: Remove or actually useisMenuActive/actionSectionRefto avoid dead state
isMenuActiveis toggled on the trigger click and reset inonMenuClose, but it isn’t used to controlCustomMenuor to drive any styling; likewise,actionSectionRefis never read. That’s extra state/refs with no behavioral effect and can be confusing.Either wire
isMenuActiveinto the trigger’s classes (e.g., to show an “open” state) or simplify by removing the unused state and callbacks, for example:-import { useState, useRef } from "react"; +import { useRef } from "react"; - // states - const [isMenuActive, setIsMenuActive] = useState(false); // translation const { t } = useTranslation(); // refs const actionSectionRef = useRef<HTMLDivElement | null>(null); // router const navigate = useNavigate(); @@ customButton={ <span ref={actionSectionRef} className="grid place-items-center p-0.5 text-custom-sidebar-text-400 hover:bg-custom-sidebar-background-80 rounded" - onClick={() => setIsMenuActive(!isMenuActive)} > <MoreHorizontal className="size-4" /> </span> } @@ - closeOnSelect - onMenuClose={() => setIsMenuActive(false)} + closeOnSelectIf you intend to keep
isMenuActivefor styling/analytics, consider adding the relevant usage now to avoid this looking like dead code.Also applies to: 45-59
apps/web/core/components/navigation/tab-navigation-overflow-menu.tsx (2)
22-27: Update the doc comment to match the current icons/behaviorThe comment still refers to “Eye” and “Star” icons, but the implementation now uses
Pinfor showing hidden tabs andSetAsDefaultIconfor default state. Consider updating this comment so it doesn’t mislead future readers.
47-57: Either style active items or dropitemIsActivefrom the key
itemIsActiveis computed but only used to change thekey(...-activevs...-inactive). That forces unmount/mount cycles when the active state flips without actually changing styles.Either use
itemIsActiveto adjust classes (e.g., text/background color) or simplify the key:- <Menu.MenuItem key={`${item.key}-overflow-${itemIsActive ? "active" : "inactive"}`} className="p-0 w-full"> + <Menu.MenuItem key={`${item.key}-overflow`} className="p-0 w-full">This keeps behavior simpler and avoids unnecessary remounts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/web/ce/components/navigations/top-navigation-root.tsx(1 hunks)apps/web/ce/components/workspace/sidebar/helper.tsx(2 hunks)apps/web/core/components/navigation/customize-navigation-dialog.tsx(7 hunks)apps/web/core/components/navigation/project-actions-menu.tsx(3 hunks)apps/web/core/components/navigation/project-header.tsx(2 hunks)apps/web/core/components/navigation/tab-navigation-overflow-menu.tsx(4 hunks)apps/web/core/components/navigation/tab-navigation-root.tsx(1 hunks)apps/web/core/components/navigation/tab-navigation-visible-item.tsx(2 hunks)apps/web/core/components/navigation/top-nav-power-k.tsx(1 hunks)apps/web/core/components/navigation/use-tab-preferences.ts(1 hunks)apps/web/core/components/workspace/sidebar/workspace-menu-root.tsx(1 hunks)packages/propel/src/icons/index.ts(1 hunks)packages/propel/src/icons/set-as-default-icon.tsx(1 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:
packages/propel/src/icons/set-as-default-icon.tsxapps/web/ce/components/workspace/sidebar/helper.tsxpackages/propel/src/icons/index.tsapps/web/ce/components/navigations/top-navigation-root.tsxapps/web/core/components/navigation/tab-navigation-root.tsxapps/web/core/components/navigation/tab-navigation-visible-item.tsxapps/web/core/components/navigation/use-tab-preferences.tsapps/web/core/components/workspace/sidebar/workspace-menu-root.tsxapps/web/core/components/navigation/tab-navigation-overflow-menu.tsxapps/web/core/components/navigation/project-header.tsxapps/web/core/components/navigation/project-actions-menu.tsxapps/web/core/components/navigation/customize-navigation-dialog.tsxapps/web/core/components/navigation/top-nav-power-k.tsx
🧠 Learnings (7)
📚 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/navigation/tab-navigation-visible-item.tsxapps/web/core/components/navigation/tab-navigation-overflow-menu.tsxapps/web/core/components/navigation/project-header.tsxapps/web/core/components/navigation/project-actions-menu.tsx
📚 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/navigation/tab-navigation-visible-item.tsxapps/web/core/components/navigation/tab-navigation-overflow-menu.tsxapps/web/core/components/navigation/project-header.tsxapps/web/core/components/navigation/project-actions-menu.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/navigation/project-header.tsx
📚 Learning: 2025-05-14T13:16:23.323Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
Applied to files:
apps/web/core/components/navigation/project-header.tsx
📚 Learning: 2025-10-09T22:12:26.424Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T22:12:26.424Z
Learning: When `types/react` is installed in a TypeScript project (which is standard for React + TypeScript codebases), React types (React.FC, React.ReactNode, React.ComponentProps, etc.) are globally available by design. These type annotations can and should be used without explicitly importing the React namespace. This is a TypeScript/DefinitelyTyped feature, not codebase-specific configuration.
Applied to files:
apps/web/core/components/navigation/project-header.tsxapps/web/core/components/navigation/project-actions-menu.tsx
📚 Learning: 2025-11-25T10:18:05.162Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.162Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use copying array methods (`toSorted`, `toSpliced`, `with`) for immutable array operations (TypeScript 5.2+)
Applied to files:
apps/web/core/components/navigation/project-actions-menu.tsx
📚 Learning: 2025-09-02T08:14:49.260Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7697
File: apps/web/app/(all)/[workspaceSlug]/(projects)/header.tsx:12-13
Timestamp: 2025-09-02T08:14:49.260Z
Learning: The star-us-link.tsx file in apps/web/app/(all)/[workspaceSlug]/(projects)/ already has "use client" directive at the top, making it a proper Client Component for hook usage.
Applied to files:
apps/web/core/components/navigation/project-actions-menu.tsx
🧬 Code graph analysis (7)
apps/web/ce/components/workspace/sidebar/helper.tsx (1)
packages/propel/src/icons/workspace/multiple-sticky-icon.tsx (1)
MultipleStickyIcon(6-13)
apps/web/ce/components/navigations/top-navigation-root.tsx (2)
apps/web/core/components/workspace/sidebar/workspace-menu-root.tsx (1)
WorkspaceMenuRoot(31-218)apps/web/core/components/navigation/top-nav-power-k.tsx (1)
TopNavPowerK(18-289)
apps/web/core/components/navigation/tab-navigation-root.tsx (3)
apps/web/core/components/navigation/project-header.tsx (1)
ProjectHeader(13-20)apps/web/core/components/navigation/project-actions-menu.tsx (1)
ProjectActionsMenu(24-114)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/components/navigation/tab-navigation-visible-item.tsx (1)
packages/propel/src/icons/set-as-default-icon.tsx (1)
SetAsDefaultIcon(3-23)
apps/web/core/components/navigation/use-tab-preferences.ts (1)
packages/propel/src/toast/toast.tsx (1)
setToast(202-222)
apps/web/core/components/navigation/project-actions-menu.tsx (1)
packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)
apps/web/core/components/navigation/customize-navigation-dialog.tsx (1)
packages/ui/src/form-fields/checkbox.tsx (1)
Checkbox(88-88)
⏰ 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). (4)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (10)
apps/web/ce/components/workspace/sidebar/helper.tsx (1)
1-12: MultipleStickyIcon import matches existing icon patternImporting
MultipleStickyIconfrom the same barrel as the other workspace icons is consistent and looks good.apps/web/core/components/workspace/sidebar/workspace-menu-root.tsx (1)
68-69: LGTM! Correct dependency array for the useEffect.The consolidated toggle logic is cleaner, and including
toggleAnySidebarDropdownin the dependency array correctly satisfies React's exhaustive-deps rule. Since it's from a MobX store hook, it should be stable and won't cause infinite re-renders.apps/web/core/components/navigation/customize-navigation-dialog.tsx (1)
74-98: LGTM!The refactored
workspaceItemsuseMemo correctly consolidates the workspace item derivation. The permission filtering, preference mapping with sensible defaults, and sorting bysortOrderare all implemented properly.apps/web/core/components/navigation/project-header.tsx (1)
13-19: Container layout simplification looks goodDirect Tailwind classes on the header container keep behavior clear and still give the name block room to truncate; no functional or type issues here.
packages/propel/src/icons/index.ts (1)
57-57: New icon export is consistent with existing icon APIRe‑exporting
./set-as-default-iconmatches the rest of the icons index pattern and cleanly exposesSetAsDefaultIconto consumers.apps/web/core/components/navigation/tab-navigation-visible-item.tsx (1)
3-10: Context menu icon additions and layout look goodAdding
SetAsDefaultIconandPinOffwith the shared flex/gap styling gives the menu clearer affordances without altering behavior. Handlers and existing labels are preserved, so the change is safely UI-only.Also applies to: 53-72
packages/propel/src/icons/set-as-default-icon.tsx (1)
1-23: Icon component implementation is clean and consistent
SetAsDefaultIconfollows the existing icon pattern (typed props,text-currentdefault, spread...restonto<svg>), so it should integrate smoothly with the rest of the icon set.apps/web/ce/components/navigations/top-navigation-root.tsx (1)
17-35: Top nav layout changes create a cleaner left/center/right distributionUsing
flex-1on the workspace and right‑actions containers with a fixed‑width Cmd‑K block (shrink-0) should give a more stable top bar layout across display modes, while the conditional padding viacnkeeps the compact look when labels are hidden. No functional concerns here.apps/web/core/components/navigation/project-actions-menu.tsx (1)
12-22: Props shape and new callbacks look consistent with usageThe new
Propstype (includingonCopyText,onLeaveProject,onPublishModal) matches how the component is used below and keeps the project dependency minimal ({ id: string }). No issues here.apps/web/core/components/navigation/tab-navigation-overflow-menu.tsx (1)
14-20: Props definition matches component usageThe new
Propstype (withoverflowItems,isActive,tabPreferences,onToggleDefault,onShow) aligns with how the component is used and keeps the surface area clear. No problems here.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/web/core/components/navigation/tab-navigation-overflow-menu.tsx (2)
69-73: Localize the “Show” label/title for hidden tabsThe
"Show"titleis hard-coded while the rest of the component usest(...). For consistency and i18n coverage, route this through the translation hook (and optionally addaria-labelfor better a11y).- className="invisible group-hover/menu-item:visible p-1 rounded text-custom-text-300 hover:text-custom-text-100 transition-colors" - title="Show" + className="invisible group-hover/menu-item:visible p-1 rounded text-custom-text-300 hover:text-custom-text-100 transition-colors" + title={t("tab_navigation.show")} + aria-label={t("tab_navigation.show")}(Adjust the key to match your existing namespace.)
75-92: Localize “Clear default” / “Set as default” stringsThe default-toggle button uses hard-coded English strings in both
tooltipContentandtitle. Since this component already usesuseTranslation, these should go throught(...)for i18n consistency; you can also share the same keys between tooltip and title.- <Tooltip tooltipContent={isDefault ? "Clear default" : "Set as default"}> + <Tooltip + tooltipContent={ + isDefault + ? t("tab_navigation.clear_default") + : t("tab_navigation.set_default") + } + > <button @@ - className={cn( + className={cn( "invisible group-hover/menu-item:visible p-1 rounded text-custom-text-300 hover:text-custom-text-100 transition-colors", { visible: isDefault, } )} - title={isDefault ? "Clear default" : "Set as default"} + title={ + isDefault + ? t("tab_navigation.clear_default") + : t("tab_navigation.set_default") + } + aria-label={ + isDefault + ? t("tab_navigation.clear_default") + : t("tab_navigation.set_default") + } >(Adjust translation keys to match your locale files.)
🧹 Nitpick comments (1)
apps/web/core/components/navigation/tab-navigation-overflow-menu.tsx (1)
54-56: Avoid encoding active state into the React keyIncluding
itemIsActivein thekeywill unmount/remount the menu item whenever active state changes, which is usually unnecessary and can reset internal state/DOM. A stable key based only onitem.keyshould be enough.- <Menu.MenuItem key={`${item.key}-overflow-${itemIsActive ? "active" : "inactive"}`} className="p-0 w-full"> + <Menu.MenuItem key={`${item.key}-overflow`} className="p-0 w-full">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/navigation/tab-navigation-overflow-menu.tsx(4 hunks)apps/web/core/components/navigation/use-tab-preferences.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/core/components/navigation/use-tab-preferences.ts
🧰 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/tab-navigation-overflow-menu.tsx
🧠 Learnings (2)
📚 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/navigation/tab-navigation-overflow-menu.tsx
📚 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/navigation/tab-navigation-overflow-menu.tsx
⏰ 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). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
Description
This PR addresses some UI bugs in the revamped navigation.
Type of Change
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.