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 @@ -116,6 +116,7 @@ export function WorkspaceSidebar({
workspaceId,
gitStatus,
onSelectFile: onSelectDiffFile,
onOpenFile: onSelectFile,
});
const changesTab: SidebarTabDefinition = {
...changesTabDef,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ interface ChangesFileListProps {
isLoading?: boolean;
worktreePath?: string;
onSelectFile?: (path: string, openInNewTab?: boolean) => void;
onOpenFile?: (absolutePath: string, openInNewTab?: boolean) => void;
onOpenInEditor?: (path: string) => void;
}

Expand All @@ -15,6 +16,7 @@ export const ChangesFileList = memo(function ChangesFileList({
isLoading,
worktreePath,
onSelectFile,
onOpenFile,
onOpenInEditor,
}: ChangesFileListProps) {
if (isLoading) {
Expand All @@ -41,6 +43,7 @@ export const ChangesFileList = memo(function ChangesFileList({
file={file}
worktreePath={worktreePath}
onSelect={onSelectFile}
onOpenFile={onOpenFile}
onOpenInEditor={onOpenInEditor}
/>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@ import {
ContextMenuShortcut,
ContextMenuTrigger,
} from "@superset/ui/context-menu";
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuShortcut,
DropdownMenuTrigger,
} from "@superset/ui/dropdown-menu";
import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { ChevronDown } from "lucide-react";
import { memo } from "react";
import { StatusIndicator } from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/StatusIndicator";
import { PathActionsMenuItems } from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/PathActionsMenuItems";
Expand All @@ -33,13 +41,15 @@ interface FileRowProps {
file: ChangesetFile;
worktreePath?: string;
onSelect?: (path: string, openInNewTab?: boolean) => void;
onOpenFile?: (absolutePath: string, openInNewTab?: boolean) => void;
onOpenInEditor?: (path: string) => void;
}

export const FileRow = memo(function FileRow({
file,
worktreePath,
onSelect,
onOpenFile,
onOpenInEditor,
}: FileRowProps) {
const { dir, basename } = splitPath(file.path);
Expand All @@ -52,46 +62,90 @@ export const FileRow = memo(function FileRow({
: undefined;

const rowButton = (
<button
type="button"
className="flex w-full items-center gap-1.5 py-1 pr-3 pl-3 text-left text-xs hover:bg-accent/50"
onClick={(e) => {
const intent = getSidebarClickIntent(e);
if (intent === "openInEditor") {
onOpenInEditor?.(file.path);
} else {
onSelect?.(file.path, intent === "openInNewTab");
}
}}
>
<FileIcon fileName={basename} className="size-3.5 shrink-0" />
<span className="flex min-w-0 flex-1 items-baseline overflow-hidden">
{dir && <span className="truncate text-muted-foreground">{dir}</span>}
{oldBasename && (
<span className="truncate text-muted-foreground">
{oldBasename}
<span className="px-1">→</span>
<div className="group relative">
<button
type="button"
className="flex w-full items-center gap-1.5 py-1 pr-3 pl-3 text-left text-xs hover:bg-accent/50"
onClick={(e) => {
const intent = getSidebarClickIntent(e);
if (intent === "openInEditor") {
onOpenInEditor?.(file.path);
} else {
onSelect?.(file.path, intent === "openInNewTab");
}
}}
>
<FileIcon fileName={basename} className="size-3.5 shrink-0" />
<span className="flex min-w-0 flex-1 items-baseline overflow-hidden">
{dir && <span className="truncate text-muted-foreground">{dir}</span>}
{oldBasename && (
<span className="truncate text-muted-foreground">
{oldBasename}
<span className="px-1">→</span>
</span>
)}
<span className="min-w-[120px] truncate font-medium text-foreground">
{basename}
</span>
)}
<span className="min-w-[120px] truncate font-medium text-foreground">
{basename}
</span>
</span>
<span className="ml-auto flex shrink-0 items-center gap-1.5">
{(file.additions > 0 || file.deletions > 0) && (
<span className="text-[10px] text-muted-foreground">
{file.additions > 0 && (
<span className="text-green-400">+{file.additions}</span>
)}
{file.additions > 0 && file.deletions > 0 && " "}
{file.deletions > 0 && (
<span className="text-red-400">-{file.deletions}</span>
)}
</span>
)}
<StatusIndicator status={file.status} />
</span>
</button>
<span className="ml-auto flex shrink-0 items-center gap-1.5 group-hover:invisible">
{(file.additions > 0 || file.deletions > 0) && (
<span className="text-[10px] text-muted-foreground">
{file.additions > 0 && (
<span className="text-green-400">+{file.additions}</span>
)}
{file.additions > 0 && file.deletions > 0 && " "}
{file.deletions > 0 && (
<span className="text-red-400">-{file.deletions}</span>
)}
</span>
)}
<StatusIndicator status={file.status} />
</span>
Comment on lines +91 to +104
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.

P2 Stats visible alongside dropdown button when menu is open off-row

The stats span uses group-hover:invisible to hide when the row is hovered, but once the dropdown is open and the pointer moves onto the menu content (a portal outside the row), the .group div is no longer hovered. At that point the stats reappear while the dropdown button stays visible via has-[[data-state=open]], so both are shown simultaneously.

Adding group-has-[[data-state=open]]:invisible to the stats span keeps them hidden for the full lifetime of the dropdown, matching the PR intent ("status badge / +/- counts hide while the action is showing"):

Suggested change
<span className="ml-auto flex shrink-0 items-center gap-1.5 group-hover:invisible">
{(file.additions > 0 || file.deletions > 0) && (
<span className="text-[10px] text-muted-foreground">
{file.additions > 0 && (
<span className="text-green-400">+{file.additions}</span>
)}
{file.additions > 0 && file.deletions > 0 && " "}
{file.deletions > 0 && (
<span className="text-red-400">-{file.deletions}</span>
)}
</span>
)}
<StatusIndicator status={file.status} />
</span>
<span className="ml-auto flex shrink-0 items-center gap-1.5 group-hover:invisible group-has-[[data-state=open]]:invisible">
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsx
Line: 91-104

Comment:
**Stats visible alongside dropdown button when menu is open off-row**

The stats span uses `group-hover:invisible` to hide when the row is hovered, but once the dropdown is open and the pointer moves onto the menu content (a portal outside the row), the `.group` div is no longer hovered. At that point the stats reappear while the dropdown button stays visible via `has-[[data-state=open]]`, so both are shown simultaneously.

Adding `group-has-[[data-state=open]]:invisible` to the stats span keeps them hidden for the full lifetime of the dropdown, matching the PR intent ("status badge / +/- counts hide while the action is showing"):

```suggestion
				<span className="ml-auto flex shrink-0 items-center gap-1.5 group-hover:invisible group-has-[[data-state=open]]:invisible">
```

How can I resolve this? If you propose a fix, please make it concise.

</button>
<div className="pointer-events-none absolute inset-y-0 right-2 flex items-center gap-0.5 opacity-0 group-hover:pointer-events-auto group-hover:opacity-100 has-[[data-state=open]]:pointer-events-auto has-[[data-state=open]]:opacity-100">
<DropdownMenu>
<DropdownMenuTrigger asChild>
<button
type="button"
aria-label="More actions"
className="flex size-5 items-center justify-center rounded text-muted-foreground hover:bg-accent hover:text-foreground data-[state=open]:bg-accent data-[state=open]:text-foreground"
onClick={(e) => e.stopPropagation()}
>
<ChevronDown className="size-3.5" />
</button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="w-56">
<DropdownMenuItem onSelect={() => onSelect?.(file.path)}>
Open Diff
</DropdownMenuItem>
<DropdownMenuItem onSelect={() => onSelect?.(file.path, true)}>
Open Diff in New Tab
<DropdownMenuShortcut>{SHIFT_CLICK_LABEL}</DropdownMenuShortcut>
</DropdownMenuItem>
<DropdownMenuItem
onSelect={() => absolutePath && onOpenFile?.(absolutePath)}
disabled={!onOpenFile || !absolutePath}
>
Open File
</DropdownMenuItem>
<DropdownMenuItem
onSelect={() => absolutePath && onOpenFile?.(absolutePath, true)}
disabled={!onOpenFile || !absolutePath}
>
Open File in New Tab
</DropdownMenuItem>
<DropdownMenuItem
onSelect={() => onOpenInEditor?.(file.path)}
disabled={!onOpenInEditor}
>
Open in Editor
<DropdownMenuShortcut>{MOD_CLICK_LABEL}</DropdownMenuShortcut>
</DropdownMenuItem>
</DropdownMenuContent>
Comment on lines +118 to +145
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.

P2 No visual separator between "Open Diff" and "Open File" action groups

The dropdown now mixes two conceptually different action groups — diff-opening actions ("Open Diff", "Open Diff in New Tab") and file-opening actions ("Open File", "Open File in New Tab") — with no visual separator between them. The existing code already uses ContextMenuSeparator before PathActionsMenuItems; a DropdownMenuSeparator (and ContextMenuSeparator) between the diff and file groups would make the menu easier to scan.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsx
Line: 118-145

Comment:
**No visual separator between "Open Diff" and "Open File" action groups**

The dropdown now mixes two conceptually different action groups — diff-opening actions ("Open Diff", "Open Diff in New Tab") and file-opening actions ("Open File", "Open File in New Tab") — with no visual separator between them. The existing code already uses `ContextMenuSeparator` before `PathActionsMenuItems`; a `DropdownMenuSeparator` (and `ContextMenuSeparator`) between the diff and file groups would make the menu easier to scan.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

</DropdownMenu>
</div>
</div>
);

return (
Expand All @@ -110,6 +164,18 @@ export const FileRow = memo(function FileRow({
Open Diff in New Tab
<ContextMenuShortcut>{SHIFT_CLICK_LABEL}</ContextMenuShortcut>
</ContextMenuItem>
<ContextMenuItem
onSelect={() => absolutePath && onOpenFile?.(absolutePath)}
disabled={!onOpenFile || !absolutePath}
>
Open File
</ContextMenuItem>
<ContextMenuItem
onSelect={() => absolutePath && onOpenFile?.(absolutePath, true)}
disabled={!onOpenFile || !absolutePath}
>
Open File in New Tab
</ContextMenuItem>
<ContextMenuItem
onSelect={() => onOpenInEditor?.(file.path)}
disabled={!onOpenInEditor}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface ChangesTabContentProps {
totalDeletions: number;
worktreePath?: string;
onSelectFile?: (path: string, openInNewTab?: boolean) => void;
onOpenFile?: (absolutePath: string, openInNewTab?: boolean) => void;
onOpenInEditor?: (path: string) => void;
onFilterChange: (filter: ChangesFilter) => void;
onBaseBranchChange: (branchName: string) => void;
Expand All @@ -44,6 +45,7 @@ export const ChangesTabContent = memo(function ChangesTabContent({
totalDeletions,
worktreePath,
onSelectFile,
onOpenFile,
onOpenInEditor,
onFilterChange,
onBaseBranchChange,
Expand Down Expand Up @@ -92,6 +94,7 @@ export const ChangesTabContent = memo(function ChangesTabContent({
isLoading={isLoading}
worktreePath={worktreePath}
onSelectFile={onSelectFile}
onOpenFile={onOpenFile}
onOpenInEditor={onOpenInEditor}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ interface UseChangesTabParams {
workspaceId: string;
gitStatus: ReturnType<typeof useGitStatus>;
onSelectFile?: (path: string, openInNewTab?: boolean) => void;
onOpenFile?: (absolutePath: string, openInNewTab?: boolean) => void;
}

export function useChangesTab({
workspaceId,
gitStatus: status,
onSelectFile,
onOpenFile,
}: UseChangesTabParams): SidebarTabDefinition {
const collections = useCollections();
const utils = workspaceTrpc.useUtils();
Expand Down Expand Up @@ -178,6 +180,7 @@ export function useChangesTab({
totalDeletions={totalDeletions}
worktreePath={worktreePath}
onSelectFile={onSelectFile}
onOpenFile={onOpenFile}
onOpenInEditor={handleOpenInEditor}
onFilterChange={setFilter}
onBaseBranchChange={setBaseBranch}
Expand Down
Loading