Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ import { Button } from "@superset/ui/button";
import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { cn } from "@superset/ui/utils";
import { useEffect, useRef, useState } from "react";
import { useDrop } from "react-dnd";
import { HiMiniXMark } from "react-icons/hi2";
import { MosaicDragType } from "react-mosaic-component";
import { StatusIndicator } from "renderer/screens/main/components/StatusIndicator";
import { useDraggingPaneStore } from "renderer/stores/tabs/dragging-pane";
import type { PaneStatus, Tab } from "renderer/stores/tabs/types";
import { getTabDisplayName } from "renderer/stores/tabs/utils";

Expand All @@ -14,6 +17,7 @@ interface GroupItemProps {
onSelect: () => void;
onClose: () => void;
onRename: (newName: string) => void;
onPaneDrop?: (paneId: string) => void;
}

export function GroupItem({
Expand All @@ -23,12 +27,43 @@ export function GroupItem({
onSelect,
onClose,
onRename,
onPaneDrop,
}: GroupItemProps) {
const displayName = getTabDisplayName(tab);
const [isEditing, setIsEditing] = useState(false);
const [editValue, setEditValue] = useState("");
const inputRef = useRef<HTMLInputElement>(null);

const [{ isOver, canDrop }, dropRef] = useDrop<
unknown,
{ handled: true },
{ isOver: boolean; canDrop: boolean }
>(
() => ({
accept: MosaicDragType.WINDOW,
drop: () => {
// Get fresh state at drop time to avoid stale closures
const { draggingPaneId, draggingTabId, setDraggingPane } =
useDraggingPaneStore.getState();
if (draggingPaneId && onPaneDrop && draggingTabId !== tab.id) {
onPaneDrop(draggingPaneId);
}
setDraggingPane(null, null);
return { handled: true };
},
canDrop: () => {
const { draggingPaneId, draggingTabId } =
useDraggingPaneStore.getState();
return !!onPaneDrop && !!draggingPaneId && draggingTabId !== tab.id;
},
collect: (monitor) => ({
isOver: monitor.isOver(),
canDrop: monitor.canDrop(),
}),
}),
[onPaneDrop, tab.id],
);

useEffect(() => {
if (isEditing && inputRef.current) {
inputRef.current.focus();
Expand Down Expand Up @@ -66,8 +101,18 @@ export function GroupItem({
: "text-muted-foreground/70 hover:text-muted-foreground hover:bg-tertiary/20",
);

const isDragActive = isOver && canDrop;

return (
<div className="group relative flex items-center shrink-0 h-full border-r border-border">
<div
ref={(node) => {
dropRef(node);
}}
className={cn(
"group relative flex items-center shrink-0 h-full border-r border-border transition-colors",
isDragActive && "bg-accent/50 ring-2 ring-inset ring-accent",
)}
>
{isEditing ? (
<div className={tabStyles}>
<input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,25 @@ import {
DropdownMenuTrigger,
} from "@superset/ui/dropdown-menu";
import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { cn } from "@superset/ui/utils";
import { useNavigate, useParams } from "@tanstack/react-router";
import { useMemo, useState } from "react";
import { useEffect, useMemo, useState } from "react";
import { useDragLayer, useDrop } from "react-dnd";
import {
HiMiniChevronDown,
HiMiniCog6Tooth,
HiMiniCommandLine,
HiMiniPlus,
HiStar,
} from "react-icons/hi2";
import { MosaicDragType } from "react-mosaic-component";
import {
getPresetIcon,
useIsDarkTheme,
} from "renderer/assets/app-icons/preset-icons";
import { HotkeyTooltipContent } from "renderer/components/HotkeyTooltipContent";
import { usePresets } from "renderer/react-query/presets";
import { useDraggingPaneStore } from "renderer/stores/tabs/dragging-pane";
import { useTabsStore } from "renderer/stores/tabs/store";
import { useTabsWithPresets } from "renderer/stores/tabs/useTabsWithPresets";
import { resolveActiveTabIdForWorkspace } from "renderer/stores/tabs/utils";
Expand All @@ -46,6 +50,22 @@ export function GroupStrip() {
const navigate = useNavigate();
const [dropdownOpen, setDropdownOpen] = useState(false);

// Monitor global drag state to clear stale drag pane state
// This handles edge cases where onDragEnd doesn't fire (e.g., source unmounts)
const { isDragging } = useDragLayer((monitor) => ({
isDragging: monitor.isDragging(),
}));

useEffect(() => {
if (!isDragging) {
const { draggingPaneId, setDraggingPane } =
useDraggingPaneStore.getState();
if (draggingPaneId) {
setDraggingPane(null, null);
}
}
}, [isDragging]);

const tabs = useMemo(
() =>
activeWorkspaceId
Expand All @@ -64,7 +84,6 @@ export function GroupStrip() {
});
}, [activeWorkspaceId, activeTabIds, allTabs, tabHistoryStacks]);

// Compute aggregate status per tab using shared priority logic
const tabStatusMap = useMemo(() => {
const result = new Map<string, ActivePaneStatus>();
for (const pane of Object.values(panes)) {
Expand Down Expand Up @@ -116,8 +135,103 @@ export function GroupStrip() {
renameTab(tabId, newName);
};

const resolveDropPaneId = () => {
const { draggingPaneId, draggingTabId } = useDraggingPaneStore.getState();
const {
activeTabIds: currentActiveTabIds,
tabHistoryStacks: currentTabHistoryStacks,
tabs: currentTabs,
panes: currentPanes,
focusedPaneIds: currentFocusedPaneIds,
} = useTabsStore.getState();

if (draggingPaneId) {
const pane = currentPanes[draggingPaneId];
if (!draggingTabId || pane?.tabId === draggingTabId) {
return draggingPaneId;
}
}

if (!activeWorkspaceId) return null;
const activeTabIdForWorkspace = resolveActiveTabIdForWorkspace({
workspaceId: activeWorkspaceId,
tabs: currentTabs,
activeTabIds: currentActiveTabIds,
tabHistoryStacks: currentTabHistoryStacks,
});
if (!activeTabIdForWorkspace) return null;
return currentFocusedPaneIds[activeTabIdForWorkspace] ?? null;
};

// Get fresh state at call time to avoid stale closures
const handlePaneDropToTab = (paneId: string, targetTabId: string) => {
const { panes, tabs, movePaneToTab } = useTabsStore.getState();
const pane = panes[paneId];
if (!pane || pane.tabId === targetTabId) return;

const targetTab = tabs.find((t) => t.id === targetTabId);
const sourceTab = tabs.find((t) => t.id === pane.tabId);
if (!targetTab || !sourceTab) return;
if (targetTab.workspaceId !== sourceTab.workspaceId) return;

movePaneToTab(paneId, targetTabId);
};

const [{ isOver: isOverStrip, canDrop: canDropStrip }, stripDropRef] =
useDrop<unknown, void, { isOver: boolean; canDrop: boolean }>(
() => ({
accept: MosaicDragType.WINDOW,
drop: (_item, monitor) => {
// Skip if a nested drop target (GroupItem) already handled it
if (monitor.didDrop()) return;
if (!monitor.isOver({ shallow: true })) return;

// Get fresh state at drop time to avoid stale closures
const { setDraggingPane } = useDraggingPaneStore.getState();
const paneId = resolveDropPaneId();
if (!paneId) return;

const { panes, tabs, movePaneToNewTab } = useTabsStore.getState();
const pane = panes[paneId];
if (!pane) return;

const sourceTab = tabs.find((t) => t.id === pane.tabId);
if (sourceTab?.workspaceId !== activeWorkspaceId) return;

movePaneToNewTab(paneId);
setDraggingPane(null, null);
},
canDrop: () => {
const paneId = resolveDropPaneId();
if (!paneId) return false;

const { panes, tabs } = useTabsStore.getState();
const pane = panes[paneId];
if (!pane) return false;

const sourceTab = tabs.find((t) => t.id === pane.tabId);
return sourceTab?.workspaceId === activeWorkspaceId;
},
collect: (monitor) => ({
isOver: monitor.isOver({ shallow: true }),
canDrop: monitor.canDrop(),
}),
}),
[activeWorkspaceId], // Only stable values in deps
);

const isStripDropActive = isOverStrip && canDropStrip;

return (
<div className="flex items-center h-10 flex-1 min-w-0">
<div
ref={(node) => {
stripDropRef(node);
}}
className={cn(
"flex items-center h-10 flex-1 min-w-0 transition-colors",
isStripDropActive && "bg-accent/30",
)}
>
{tabs.length > 0 && (
<div
className="flex items-center h-full overflow-x-auto overflow-y-hidden border-l border-border pr-2"
Expand All @@ -136,6 +250,7 @@ export function GroupStrip() {
onSelect={() => handleSelectGroup(tab.id)}
onClose={() => handleCloseGroup(tab.id)}
onRename={(newName) => handleRenameGroup(tab.id, newName)}
onPaneDrop={(paneId) => handlePaneDropToTab(paneId, tab.id)}
/>
</div>
))}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useRef } from "react";
import { useCallback, useRef } from "react";
import type { MosaicBranch } from "react-mosaic-component";
import { MosaicWindow } from "react-mosaic-component";
import { useDraggingPaneStore } from "renderer/stores/tabs/dragging-pane";
import type { SplitOrientation } from "../../hooks";
import { useSplitOrientation } from "../../hooks";

Expand Down Expand Up @@ -43,6 +44,16 @@ export function BasePaneWindow({
}: BasePaneWindowProps) {
const containerRef = useRef<HTMLDivElement>(null);
const splitOrientation = useSplitOrientation(containerRef);
const setDraggingPane = useDraggingPaneStore((s) => s.setDraggingPane);

const handleDragStart = useCallback(() => {
setFocusedPane(tabId, paneId);
setDraggingPane(paneId, tabId);
}, [paneId, tabId, setDraggingPane, setFocusedPane]);

const handleDragEnd = useCallback(() => {
setDraggingPane(null, null);
}, [setDraggingPane]);

const handleFocus = () => {
setFocusedPane(tabId, paneId);
Expand Down Expand Up @@ -75,6 +86,8 @@ export function BasePaneWindow({
title=""
renderToolbar={() => renderToolbar(handlers)}
className={isActive ? "mosaic-window-focused" : ""}
onDragStart={handleDragStart}
onDragEnd={handleDragEnd}
>
{/* biome-ignore lint/a11y/useKeyWithClickEvents lint/a11y/noStaticElementInteractions: Focus handler for pane */}
<div
Expand Down
14 changes: 14 additions & 0 deletions apps/desktop/src/renderer/stores/tabs/dragging-pane.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { create } from "zustand";

interface DraggingPaneState {
draggingPaneId: string | null;
draggingTabId: string | null;
setDraggingPane: (paneId: string | null, tabId: string | null) => void;
}

export const useDraggingPaneStore = create<DraggingPaneState>((set) => ({
draggingPaneId: null,
draggingTabId: null,
setDraggingPane: (paneId, tabId) =>
set({ draggingPaneId: paneId, draggingTabId: tabId }),
Comment on lines +3 to +13
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use a params object for setDraggingPane to avoid positional arguments.

This API now has 2+ parameters and should follow the project convention for self-documentation and future extensibility. Update call sites accordingly. As per coding guidelines, ...

♻️ Proposed refactor
 interface DraggingPaneState {
 	draggingPaneId: string | null;
 	draggingTabId: string | null;
-	setDraggingPane: (paneId: string | null, tabId: string | null) => void;
+	setDraggingPane: (params: {
+		paneId: string | null;
+		tabId: string | null;
+	}) => void;
 }

 export const useDraggingPaneStore = create<DraggingPaneState>((set) => ({
 	draggingPaneId: null,
 	draggingTabId: null,
-	setDraggingPane: (paneId, tabId) =>
-		set({ draggingPaneId: paneId, draggingTabId: tabId }),
+	setDraggingPane: ({ paneId, tabId }) =>
+		set({ draggingPaneId: paneId, draggingTabId: tabId }),
 }));
-	setDraggingPane(paneId, tabId);
+	setDraggingPane({ paneId, tabId });
-	setDraggingPane(null, null);
+	setDraggingPane({ paneId: null, tabId: null });
🤖 Prompt for AI Agents
In `@apps/desktop/src/renderer/stores/tabs/dragging-pane.ts` around lines 3 - 13,
Change setDraggingPane to accept a single params object instead of positional
arguments: update the DraggingPaneState type so setDraggingPane: (args: {
paneId: string | null; tabId: string | null }) => void, and update the
useDraggingPaneStore initializer to implement setDraggingPane: ({ paneId, tabId
}) => set({ draggingPaneId: paneId, draggingTabId: tabId }). Rename parameters
in the implementation as shown and then find and update all call sites that
invoke useDraggingPaneStore().setDraggingPane(paneId, tabId) to the new form
setDraggingPane({ paneId, tabId }); keep the property names paneId and tabId
exactly to match the type.

}));
Loading