Fix dropping workspace nav items at the bottom of the list by abdulrahmancodes · Pull Request #18989 · twentyhq/twenty

abdulrahmancodes · 2026-03-26T07:12:27Z

Screen.Recording.2026-03-26.at.1.10.33.PM.mov

…ceSectionListDndKit - Simplified the calculation of adjusted index in computeDndReorderPosition to ensure correct item placement during drag-and-drop operations. - Enhanced WorkspaceSectionListDndKit to correctly compute the index for orphan items, ensuring proper rendering and functionality in the drag-and-drop interface.

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

cubic-dev-ai

No issues found across 2 files

- Introduced a new styled component, StyledOrphanAppendSlotOverlap, to manage layout for orphan items. - Updated NavigationMenuItemDroppableSlot to include collision priority for better item placement during drag-and-drop operations. - Refactored the rendering logic for orphan drop targets to improve clarity and maintainability.

- Added a new prop, highlightPosition, to NavigationItemDropTarget and NavigationMenuItemOrphanDropTarget to specify the position of the highlight (top or bottom). - Updated StyledDropTarget to conditionally render the highlight based on the new prop. - Adjusted WorkspaceSectionListDndKit to utilize the highlightPosition prop for improved visual feedback during drag-and-drop operations.

etiennejouan

It works well ! 👍

But nav menu items position computing seems very confusing. Sorry we should have brainstormed this earlier. It should simplify a lot to display not permitted nav menu item in edition mode, one single source of truth for nav menu item. Waiting for bonapara design.

When dragging on last position, "Add menu item" should not be highlighted (with blue background)

...es/navigation-menu-item/display/sections/workspace/components/WorkspaceSectionListDndKit.tsx

etiennejouan · 2026-03-26T16:04:57Z

...es/navigation-menu-item/display/sections/workspace/components/WorkspaceSectionListDndKit.tsx

+          }
+          index={orphanAppendDndIndex}
+          disabled={workspaceDropDisabled}
+          collisionPriority={FOLDER_HEADER_SLOT_COLLISION_PRIORITY}


nit : should not be re-used for a non-folder context. All collision priority should also be centralized

etiennejouan · 2026-03-26T16:36:21Z

locker icon on navigation menu items in edition mode, when user does not have permission. Note that as record name can't be display then "Record" is set to replace (concerning object name it's metadata, it's ok).
Concerning the Object not shared view, thomast is working on it

…rphan drop targets - Updated the compact prop in NavigationMenuItemOrphanDropTarget to always be true. - Adjusted rendering of WorkspaceSectionAddMenuItemButton to improve clarity and maintainability.

sentry · 2026-03-27T02:25:59Z

...es/navigation-menu-item/display/sections/workspace/components/WorkspaceSectionListDndKit.tsx

+          droppableId={
+            NavigationMenuItemDroppableIds.WORKSPACE_ORPHAN_NAVIGATION_MENU_ITEMS
+          }
+          index={orphanAppendDndIndex}


Bug: When dragging an item to the end of the orphan list, an unclamped destinationIndex can cause out-of-bounds access, incorrectly positioning the item at the start instead of the end.
_{Severity: HIGH}

Suggested Fix

In computeDndReorderPosition.ts, for the isSameList=false case, clamp the destinationIndex to the bounds of the sortedList before accessing its elements. This can be done by adding destinationIndex = Math.min(destinationIndex, sortedList.length); before calculating prevItem and nextItem, mirroring the protection in the isSameList=true path.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: packages/twenty-front/src/modules/navigation-menu-item/display/sections/workspace/components/WorkspaceSectionListDndKit.tsx#L100-L103 Potential issue: In the `computeDndReorderPosition` utility, when an item is dragged from a folder to the orphan list (`isSameList=false`), the destination index is not clamped. If the `destinationIndex` (derived from `orphanAppendDndIndex`) is greater than the length of the `sortedList`, array access for `prevItem` and `nextItem` will be out of bounds, resulting in `undefined` for both. This causes `getPositionBetween(undefined, undefined)` to return `0`, incorrectly placing the item at the beginning of the list instead of the end. This issue occurs because the `isSameList=false` path lacks the clamping logic (`Math.min`) present in the `isSameList=true` path.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

- Integrated layout customization mode state into useNavigationMenuItemSectionItems to conditionally include inaccessible object-backed items. - Updated NavigationDrawerItemForObjectMetadataItem to display a lock icon for inaccessible records in layout customization mode. - Adjusted WorkspaceSectionContainer and WorkspaceSectionListDndKit to handle orphan items based on layout customization mode. - Modified getWorkspaceSidebarOrphanItemsInDisplayOrder to support inclusion of inaccessible items based on the new flag.

- Introduced LayoutCustomizationObjectNotSharedEmptyState component to display a message and images when an object is not shared. - Added placeholder images for background and moving illustrations. - Updated RecordIndexContainerGater to render the new empty state when access permissions are denied and layout customization mode is not enabled. - Enhanced styling for the empty state to improve user experience.

cubic-dev-ai

2 issues found across 9 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-front/src/modules/layout-customization/components/LayoutCustomizationObjectNotSharedEmptyState.tsx">

<violation number="1" location="packages/twenty-front/src/modules/layout-customization/components/LayoutCustomizationObjectNotSharedEmptyState.tsx:85">
P3: Avoid rendering a second `PageTitle` inside this empty state; the parent page already sets the title.</violation>
</file>

<file name="packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexContainerGater.tsx">

<violation number="1" location="packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexContainerGater.tsx:69">
P1: This change makes no-read users mount record-index loading effects in layout customization mode, which can still trigger record/view loading and default-view creation API calls.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-03-27T06:48:27Z

...twenty-front/src/modules/object-record/record-index/components/RecordIndexContainerGater.tsx

  );

-  if (!hasObjectReadPermissions) {
+  if (!hasObjectReadPermissions && !isLayoutCustomizationModeEnabled) {


P1: This change makes no-read users mount record-index loading effects in layout customization mode, which can still trigger record/view loading and default-view creation API calls.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexContainerGater.tsx, line 69: <comment>This change makes no-read users mount record-index loading effects in layout customization mode, which can still trigger record/view loading and default-view creation API calls.</comment> <file context> @@ -60,7 +66,7 @@ export const RecordIndexContainerGater = () => { ); - if (!hasObjectReadPermissions) { + if (!hasObjectReadPermissions && !isLayoutCustomizationModeEnabled) { return <NotFound />; } </file context>

...src/modules/layout-customization/components/LayoutCustomizationObjectNotSharedEmptyState.tsx

…t logic for layout customization mode - Removed the pageTitle prop from LayoutCustomizationObjectNotSharedEmptyState for a cleaner implementation. - Updated RecordIndexContainerGater to reflect the changes in the empty state component. - Enhanced useExitLayoutCustomizationMode to include redirection logic based on object permissions, improving user navigation experience.

…nent structure - Reformatted the JSX structure for improved readability without changing functionality. - Ensured consistent indentation and spacing for better maintainability.

greptile-apps bot reviewed Mar 26, 2026

View reviewed changes

cubic-dev-ai bot reviewed Mar 26, 2026

View reviewed changes

abdulrahmancodes added 3 commits March 26, 2026 12:47

Fix indentation in WorkspaceSectionListDndKit for highlightPosition prop

b4e44b6

etiennejouan reviewed Mar 26, 2026

View reviewed changes

etiennejouan assigned abdulrahmancodes Mar 26, 2026

etiennejouan added the -PR: awaiting author label Mar 26, 2026

Refactor WorkspaceSectionListDndKit to simplify rendering logic for o…

4df86c3

…rphan drop targets - Updated the compact prop in NavigationMenuItemOrphanDropTarget to always be true. - Adjusted rendering of WorkspaceSectionAddMenuItemButton to improve clarity and maintainability.

sentry bot reviewed Mar 27, 2026

View reviewed changes

abdulrahmancodes added 2 commits March 27, 2026 11:20

cubic-dev-ai bot reviewed Mar 27, 2026

View reviewed changes

abdulrahmancodes added 3 commits March 27, 2026 12:27

Merge branch 'main' into fix/workspace-nav-bottom-drop-dnd

9396e3f

refactor: clean up LayoutCustomizationObjectNotSharedEmptyState compo…

d3ba83e

…nent structure - Reformatted the JSX structure for improved readability without changing functionality. - Ensured consistent indentation and spacing for better maintainability.

abdulrahmancodes requested a review from etiennejouan March 27, 2026 08:41

abdulrahmancodes added -PR: awaiting review and removed -PR: awaiting author labels Mar 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dropping workspace nav items at the bottom of the list#18989

Fix dropping workspace nav items at the bottom of the list#18989
abdulrahmancodes wants to merge 10 commits intomainfrom
fix/workspace-nav-bottom-drop-dnd

abdulrahmancodes commented Mar 26, 2026 •

edited

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

etiennejouan left a comment

Uh oh!

Uh oh!

etiennejouan Mar 26, 2026

Uh oh!

etiennejouan commented Mar 26, 2026 •

edited

Loading

Uh oh!

sentry bot Mar 27, 2026

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Mar 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

abdulrahmancodes commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

etiennejouan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

etiennejouan Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

etiennejouan commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sentry bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abdulrahmancodes commented Mar 26, 2026 •

edited

Loading

etiennejouan commented Mar 26, 2026 •

edited

Loading

cubic-dev-ai bot Mar 27, 2026 •

edited

Loading