Conversation
WalkthroughThis change introduces comprehensive support for tab and tab group reordering via drag-and-drop in the sidebar UI. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SidebarUI
participant Renderer
participant Preload
participant MainIPC
participant TabManager
User->>SidebarUI: Drag tab or tab group
SidebarUI->>Renderer: Handle drag event, show drop indicator
User->>SidebarUI: Drop tab at new position
SidebarUI->>Preload: tabsAPI.moveTab(tabId, newPosition)
Preload->>MainIPC: IPC "tabs:move-tab" (tabId, newPosition)
MainIPC->>TabManager: Move tab to new position
TabManager-->>MainIPC: Tab position updated
MainIPC-->>Preload: Success
Preload-->>SidebarUI: Success
SidebarUI->>Renderer: Update UI with new positions
sequenceDiagram
participant User
participant SidebarUI
participant Renderer
participant Preload
participant MainIPC
participant TabManager
User->>SidebarUI: Drag tab to another space
SidebarUI->>Preload: tabsAPI.moveTabToWindowSpace(tabId, spaceId, newPosition)
Preload->>MainIPC: IPC "tabs:move-tab-to-window-space" (tabId, spaceId, newPosition)
MainIPC->>TabManager: Move tab to target space and position
TabManager-->>MainIPC: Tab moved
MainIPC-->>Preload: Success
Preload-->>SidebarUI: Success
SidebarUI->>Renderer: Update UI with new tab location
Suggested labels
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
iamEvanYT
force-pushed
the
evan/reorder-tabs
branch
from
b7e95b6 to
b6c5843
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (12)
src/main/browser/tabs/tab-manager.ts (1)
655-663: Consider handling edge cases in getSmallestPosition().The method works for finding the smallest position value, but has a few edge cases:
- If there are no tabs (empty map), it returns 999
- Using 999 as an initial value assumes there won't be more tabs with higher positions
Consider this alternative implementation:
public getSmallestPosition(): number { - let smallestPosition = 999; + const tabs = Array.from(this.tabs.values()); + if (tabs.length === 0) { + return 0; // Return 0 for empty collection + } + let smallestPosition = tabs[0].position; for (const tab of this.tabs.values()) { if (tab.position < smallestPosition) { smallestPosition = tab.position; } } return smallestPosition; }src/renderer/src/components/browser-ui/sidebar/spaces-switcher.tsx (2)
45-47: Remove strayconsole.logbefore merging
console.log("clicked");will spam the dev-tools every time a space is selected through a drag gesture.- console.log("clicked");
80-85: Duplicatekeyprop – only supply it from the parent
SidebarMenuButtonreceives its ownkeybut it’s already keyed at<SpaceButton>level in the parent loop. The inner key is ignored and can be dropped:- <SidebarMenuButton - key={space.id} + <SidebarMenuButtonMinor, yet keeps JSX tidy.
src/main/browser/tabs/tab.ts (1)
69-76: Exposepositionvia the public option but document invariants
positionis now accepted inTabCreationOptions, good. Please document that callers must pass an integer and that duplicate positions are tolerated/normalised by the tab manager. This avoids misuse when external callers (IPC, tests) adopt the option.src/main/saving/tabs.ts (1)
58-59: Console log left in production path.
console.log("saving tab", tabData.id, tabData.position);
will spam dev-tools every time a tab updates.
Either gate it behind a verbose / debug flag
or replace with your project’s logger.src/main/ipc/browser/tabs.ts (1)
255-256: Remove leftover debugconsole.log.
console.log("moving tab", …)should not stay in IPC code.src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx (2)
31-36: Potential position collision: alwaysbiggestIndex + 1.If users reorder tabs repeatedly,
positionvalues may grow
without bound and eventually lose precision (they’re floats behind the
scenes).
Consider asking the backend forgetNextPosition()or use a
median-between algorithm ((prev + next) / 2) to keep numbers small.
37-44: Cross-profile drop TODO – surface to UX.The comment notes that moving tabs between profiles is unsupported.
Make sure the UI gives feedback (cursor “not-allowed” or tooltip) so
users understand why the drop is rejected.src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (2)
254-267: Drag payload includes fulltabsarray in deps – expensive re-subscriptions.
useEffectdepends ontabGroup.tabs; every tab title / mute change
re-initialises drag-and-drop listeners.
Switch totabGroup.idor memoise the array length to avoid DOM
re-registration thrashing.- }, [moveTab, tabGroup.id, position, tabGroup.tabs, tabGroup.spaceId, tabGroup.profileId]); + }, [moveTab, tabGroup.id, position, tabGroup.tabs.length, tabGroup.spaceId, tabGroup.profileId]);
215-236: Floating-point positions will accumulate tail decimals.Using
position ± 0.5after many moves produces values like
42.0000000007.
Plan for periodic normalisation (e.g., re-index to integers) or switch
to a gap-based integer scheme (…10,20,30…).src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx (2)
14-14: Typo in constant name
ENABLE_SECTION_DEVIDER→ENABLE_SECTION_DIVIDER.
Renaming prevents future confusion and aligns with the spelling used in the JSX below.-const ENABLE_SECTION_DEVIDER = true; +const ENABLE_SECTION_DIVIDER = true;
92-93: UnusedsidebarRef– remove or use
sidebarRefis created but never read. Dead refs add noise and may mislead future readers.-const sidebarRef = useRef<HTMLDivElement>(null); ... -<div ... ref={sidebarRef}> +<div ...>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
package.json(1 hunks)src/main/browser/tabs/tab-groups/index.ts(1 hunks)src/main/browser/tabs/tab-manager.ts(1 hunks)src/main/browser/tabs/tab.ts(5 hunks)src/main/ipc/browser/tabs.ts(3 hunks)src/main/saving/tabs.ts(2 hunks)src/preload/index.ts(1 hunks)src/renderer/src/components/browser-ui/browser-sidebar.tsx(1 hunks)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx(1 hunks)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx(2 hunks)src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx(2 hunks)src/renderer/src/components/browser-ui/sidebar/content/space-title.tsx(1 hunks)src/renderer/src/components/browser-ui/sidebar/spaces-switcher.tsx(2 hunks)src/renderer/src/components/providers/tabs-provider.tsx(1 hunks)src/shared/flow/interfaces/browser/tabs.ts(1 hunks)src/shared/types/tabs.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/saving/tabs.ts (3)
src/main/ipc/browser/tabs.ts (1)
getTabData(9-34)src/main/browser/tabs/tab.ts (1)
SLEEP_MODE_URL(23-23)src/shared/types/tabs.ts (1)
TabData(8-31)
src/main/browser/tabs/tab.ts (1)
src/main/browser/tabs/tab-groups/index.ts (1)
position(213-216)
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx (2)
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (1)
TabGroupSourceData(187-194)src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx (1)
DropIndicator(16-37)
src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx (4)
src/renderer/src/lib/utils.ts (1)
cn(5-7)src/renderer/src/components/browser-ui/sidebar/content/space-title.tsx (1)
SpaceTitle(5-18)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (1)
SidebarTabGroups(196-330)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx (1)
SidebarTabDropTarget(17-81)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
🔇 Additional comments (12)
package.json (1)
46-47: New dependencies for drag-and-drop functionality look good.Adding Atlaskit's drag-and-drop libraries provides a solid foundation for implementing the tab reordering feature. These libraries are well-maintained and provide good accessibility support.
src/renderer/src/components/browser-ui/sidebar/content/space-title.tsx (1)
11-11: Added padding improves visual spacing.The
py-0.5class adds slight vertical padding to the sidebar group, which helps maintain consistent spacing when implementing drag-and-drop functionality.src/renderer/src/components/providers/tabs-provider.tsx (1)
113-115: Position property properly propagated to standalone tabs.Good addition of the
positionproperty from the tab to the generated tab group. This ensures standalone tabs (not in a group) maintain their position information for correct ordering in the UI.src/main/browser/tabs/tab-groups/index.ts (1)
213-216:Details
❌ Incorrect review comment
Consider handling empty tab groups.
The position getter uses the first tab's position (
this.tabs[0].position), but doesn't handle the case wherethis.tabsmight be empty. This could happen if all tabs are removed from a group but the group hasn't been destroyed yet.Consider adding a fallback value or check:
public get position(): number { this.errorIfDestroyed(); + if (this.tabs.length === 0) { + return 0; // or another appropriate default value + } return this.tabs[0].position; }
🏁 Script executed:
#!/bin/bash # Check if there are any safeguards for empty tab groups elsewhere in the codebase rg -A 3 -B 3 "tabs.length\s*===?\s*0" --glob "*.ts*"Length of output: 1075
Position getter safe—empty groups are destroyed
- In src/main/browser/tabs/tab-manager.ts:
if (activeElement.tabs.length === 0) { this.destroyTabGroup(activeElement.id); }if (group.tabs.length === 0) { this.destroyTabGroup(group.id); }- Since any group with an empty
tabsarray is immediately destroyed anderrorIfDestroyed()prevents further access,this.tabs[0]is only ever called on non-empty groups. No additional empty-check or fallback is required.Likely an incorrect or invalid review comment.
src/shared/types/tabs.ts (2)
13-13: Well-designed type extension for tab positioning.Adding
positionto theTabDatainterface is a logical addition for supporting tab reordering functionality.
40-40: Consistent type extension for tab group positioning.The
positionfield inTabGroupDatacomplements the same field inTabData, providing a consistent approach to positioning both individual tabs and tab groups.src/renderer/src/components/browser-ui/browser-sidebar.tsx (1)
35-35: Good addition of reusable styling constant.Adding the
SIDEBAR_HOVER_COLOR_PLAINconstant follows the pattern of the existing hover color constant and provides a reusable styling element for hover states.src/shared/flow/interfaces/browser/tabs.ts (2)
57-63: Well-documented API for tab repositioning.The
moveTabmethod is well-designed with clear parameters and return type. The JSDoc comments effectively explain the purpose and parameters.
64-70: Comprehensive API for tab movement across spaces.The
moveTabToWindowSpacemethod provides a flexible way to move tabs between spaces with an optional position parameter. This allows for both simple space transfers and precise positioning.src/main/saving/tabs.ts (2)
21-24: Removal of early–exit guards may re-introduce “sleep” tabs into the DB.The three early returns that filtered out
SLEEP_MODE_URL,tab.asleep, and emptynavHistory
are now commented out.
Persisting such tabs makes the datastore grow with transient “sleep”
records that can never be re-opened (because they load
about:blank?sleep=true).Please double-check that the new
saveURL / saveNavHistory
logic really prevents this or re-enable at least one fast-path
to avoid noisy records.
140-148: 👍 Position is now persisted and restored.Passing
position: tabData.positionintocreateTab
keeps ordering intact after a restart – good catch.src/main/ipc/browser/tabs.ts (1)
220-234: Returning explicit boolean – looks good.The
tabs:set-tab-mutedhandler now returnstrue
after updating state, aligning it with other handlers.
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx
Outdated
Show resolved
Hide resolved
src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx
Outdated
Show resolved
Hide resolved
src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx
Show resolved
Hide resolved
src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx
Outdated
Show resolved
Hide resolved
src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx (5)
106-117: Performance issue: O(n³) complexity in moveTab function.The sorting logic still has the O(n³) performance issue mentioned in past reviews:
sort()is O(n log n)findIndex()inside the comparator is O(n) per comparison- This creates O(n² log n) or worse complexity
Additionally, calling
findIndex()on the array being sorted can yield unstable results. Consider the suggested refactor from the past review using a map-sort-map pattern for O(n log n) complexity.
9-9:⚠️ Potential issueFix invalid motion import path.
The import
"motion/react"is incorrect and will cause build errors. Please update to use the correct package:If using Framer Motion:
-import { AnimatePresence, motion } from "motion/react"; +import { AnimatePresence, motion } from "framer-motion";If using Motion One:
-import { AnimatePresence, motion } from "motion/react"; +import { AnimatePresence, motion } from "@motionone/react";
152-157: Verify biggestIndex calculation for edge cases.The
biggestIndexcalculation (sortedTabGroups.length - 1) can be-1when no tab groups exist, potentially causing issues downstream. Past reviews identified this concern.Verify that
SidebarTabDropTargetproperly handles the case wherebiggestIndexis-1:#!/bin/bash # Check if SidebarTabDropTarget handles negative biggestIndex values ast-grep --pattern $'biggestIndex + 1'
9-10:⚠️ Potential issueFix invalid motion import path.
The import path
"motion/react"is incorrect and will cause build errors. Based on the past review comments, this issue was previously identified.Update the import to match your installed motion library:
- import { AnimatePresence, motion } from "motion/react"; + import { AnimatePresence, motion } from "framer-motion";Ensure the correct motion library is installed in your dependencies.
104-126:⚠️ Potential issueCritical performance and logic issues in moveTab algorithm.
The
moveTabimplementation has several serious problems that were identified in past reviews but remain unaddressed:
- O(n³) complexity: Using
findIndexinside the sort comparator creates extremely poor performance- Unstable sorting: Calling
findIndexon the same array being sorted can yield unpredictable results- Logic complexity: The current approach is unnecessarily complex
Apply this refactor to fix the performance and stability issues:
- const moveTab = useCallback( - (tabId: number, newPosition: number) => { - const newSortedTabGroups = [...sortedTabGroups].sort((a, b) => { - const isTabInGroupA = a.tabs.some((tab) => tab.id === tabId); - const isTabInGroupB = b.tabs.some((tab) => tab.id === tabId); - - const aIndex = newSortedTabGroups.findIndex((tabGroup) => tabGroup.id === a.id); - const bIndex = newSortedTabGroups.findIndex((tabGroup) => tabGroup.id === b.id); - - const aPos = isTabInGroupA ? newPosition : aIndex; - const bPos = isTabInGroupB ? newPosition : bIndex; - - return aPos - bPos; - }); - - for (const [index, tabGroup] of newSortedTabGroups.entries()) { - if (tabGroup.position !== index) { - flow.tabs.moveTab(tabGroup.tabs[0].id, index); - } - } - }, - [sortedTabGroups] - ); + const moveTab = useCallback( + (tabId: number, newPosition: number) => { + // Create reordered array with O(n log n) complexity + const newSortedTabGroups = [...sortedTabGroups].map((group) => { + const containsDragged = group.tabs.some((t) => t.id === tabId); + return { + group, + sortKey: containsDragged ? newPosition : group.position + }; + }) + .sort((a, b) => a.sortKey - b.sortKey) + .map((entry) => entry.group); + + // Apply position updates + for (const [index, tabGroup] of newSortedTabGroups.entries()) { + if (tabGroup.position !== index) { + flow.tabs.moveTab(tabGroup.tabs[0].id, index); + } + } + }, + [sortedTabGroups] + );
🧹 Nitpick comments (1)
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (1)
215-310: Comprehensive drag-and-drop implementation with proper edge case handling.The drag-and-drop logic correctly handles:
- Edge detection with top/bottom positioning
- Cross-space tab movement
- Profile-based permission checks
- Position calculations using fractional values
The fix for the falsy check issue (line 240:
if (newPos !== undefined)) properly handles the case wherenewPosis0, which was correctly identified in past reviews.However, consider adding error handling for edge cases:
+ try { if (newPos !== undefined) { moveTab(sourceTabId, newPos); } if (sourceData.spaceId != tabGroup.spaceId) { if (sourceData.profileId != tabGroup.profileId) { // TODO: @MOVE_TABS_BETWEEN_PROFILES not supported yet } else { // move tab to new space flow.tabs.moveTabToWindowSpace(sourceTabId, tabGroup.spaceId, newPos); } } + } catch (error) { + console.error('Failed to move tab:', error); + // Optionally show user feedback + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/ipc/browser/tabs.ts(3 hunks)src/main/saving/tabs.ts(2 hunks)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx(1 hunks)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx(3 hunks)src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx(2 hunks)src/renderer/src/components/browser-ui/sidebar/spaces-switcher.tsx(2 hunks)src/renderer/src/index.css(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/renderer/src/index.css
🚧 Files skipped from review as they are similar to previous changes (4)
- src/renderer/src/components/browser-ui/sidebar/spaces-switcher.tsx
- src/main/saving/tabs.ts
- src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx
- src/main/ipc/browser/tabs.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx (4)
src/renderer/src/lib/utils.ts (1)
cn(5-7)src/renderer/src/components/browser-ui/sidebar/content/space-title.tsx (1)
SpaceTitle(5-18)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (1)
SidebarTabGroups(196-330)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx (1)
SidebarTabDropTarget(17-81)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (13)
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (6)
187-194: LGTM! Well-defined drag source data type.The
TabGroupSourceDatatype provides all necessary metadata for drag-and-drop operations and will ensure type safety across the drag-and-drop system.
128-128: Good addition of CSS classes for drag behavior.The
user-drag-noneclass prevents unwanted drag behavior on favicon images, andobject-containwithoverflow-hiddenensures proper image fitting.
5-15: LGTM! Appropriate imports for drag-and-drop functionality.The imports are correctly structured for the drag-and-drop implementation using Atlaskit's pragmatic-drag-and-drop library. The motion imports and React hooks are also properly imported.
128-128: Good addition of CSS classes for favicon behavior.The
user-drag-none,object-contain, andoverflow-hiddenclasses properly control the drag behavior and image rendering for the favicon, preventing unwanted drag interactions while ensuring proper image display.
187-194: Well-structured type definition for drag source data.The
TabGroupSourceDatatype provides comprehensive metadata for drag operations, including all necessary identifiers for tab group positioning and cross-space/profile operations.
312-329: Clean render logic with proper conditional indicators.The render logic correctly shows drop indicators based on the closest edge state and properly wraps the tab group container with motion animations. The
refattachment for drag-and-drop registration is correctly implemented.src/renderer/src/components/browser-ui/sidebar/content/space-sidebar.tsx (7)
100-102: Excellent memoization fix!The
useMemowrapper forsortedTabGroupsaddresses the performance concern from previous reviews and prevents unnecessary re-renders downstream.
39-70: Well-implemented animated section divider.The
SidebarSectionDividercomponent provides nice visual feedback with smooth animations and a clear "Clear" button for closing all tabs. The conditional rendering based onhasTabsis appropriate.
16-37: Clean drop indicator implementation.The
DropIndicatorcomponent properly handles theming with theisSpaceLightprop and integrates well with the Atlaskit drag-and-drop system.
16-37: Well-implemented drop indicator component.The
DropIndicatorcomponent properly uses the Atlaskit drop indicator with dynamic styling based on space theme. The CSS custom properties approach for theming is clean and effective.
39-70: Nice animated section divider with clear action.The
SidebarSectionDividercomponent provides a clean way to clear all tabs with proper animations and responsive layout. The conditional rendering based onhasTabsand sidebar state is well-implemented.
100-102: Good fix for performance issue with memoization.The
useMemohook properly addresses the performance concern raised in past reviews by preventing unnecessary recalculations of the sorted tab groups array.
129-164: Good overall layout structure with proper component integration.The render logic properly integrates all the new components with:
- Correct dark/light theme handling
- Proper flex layout for full height
- Correct prop passing to child components
- Good use of AnimatePresence for smooth transitions
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (1)
240-249: Past review issues have been properly addressed.The logic correctly avoids double operations by using
else iffor same-space moves and properly handles the position 0 case withnewPos !== undefined.
🧹 Nitpick comments (1)
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (1)
308-308: Optimize useEffect dependencies to prevent unnecessary re-renders.Including
tabGroup.tabsin the dependencies could cause the drag-and-drop setup to be recreated every time tabs change, which is unnecessary since the setup only depends on tab group metadata.Consider removing
tabGroup.tabsfrom the dependencies:- }, [moveTab, tabGroup.id, position, tabGroup.tabs, tabGroup.spaceId, tabGroup.profileId]); + }, [moveTab, tabGroup.id, position, tabGroup.spaceId, tabGroup.profileId]);The primary tab ID is already captured from
tabGroup.tabs[0].idduring drag initialization, so the tabs array doesn't need to be in dependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx(1 hunks)src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-drop-target.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
🔇 Additional comments (6)
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx (6)
5-5: LGTM: Clean import additions for drag-and-drop functionality.The imports are well-organized and include all necessary hooks and drag-and-drop utilities from the @atlaskit library.
Also applies to: 8-15
128-128: Good addition of drag control CSS classes.The
user-drag-none,object-contain, andoverflow-hiddenclasses appropriately prevent unwanted drag behavior on the favicon while ensuring proper image display.
187-194: Well-structured type definition for drag source data.The
TabGroupSourceDatatype comprehensively captures all necessary metadata for drag-and-drop operations.
234-238: Position calculation logic is correct.The fractional positioning using +/- 0.5 is a standard approach for insertion-based reordering, allowing items to be placed between existing positions.
267-302: Comprehensive drop target configuration.The drop target properly validates source compatibility, handles edge detection, and manages state updates. The
canDroplogic correctly prevents dropping on self and enforces profile restrictions.
312-312: Good UX with conditional drop indicators.The drop indicators provide clear visual feedback for where the tab group will be placed during drag operations.
Also applies to: 325-325
src/renderer/src/components/browser-ui/sidebar/content/sidebar-tab-groups.tsx
Show resolved
Hide resolved
1 participant
Summary by CodeRabbit
New Features
Enhancements
Style
Bug Fixes