Skip to content
Open
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
@@ -1,4 +1,5 @@
import { Check, Loader2 } from 'lucide-react';
import { observer } from 'mobx-react-lite';
import { useCallback, useEffect, useRef, useState } from 'react';
import type { Issue } from '@shared/tasks';
import {
Expand All @@ -10,6 +11,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger } from '@renderer/lib/
import { ShortcutHint } from '@renderer/lib/ui/shortcut-hint';
import { cn } from '@renderer/utils/utils';
import { ConnectIssueIntegrationPlaceholder, IssueRow, ProviderLogo } from './issue-selector';
import { getLinkedIssueMap } from './use-linked-issue-urls';
import { useIssueSearch } from './useIssueSearch';

export interface InlineIssueSelectorProps {
Expand All @@ -19,16 +21,20 @@ export interface InlineIssueSelectorProps {
repositoryUrl?: string;
projectPath?: string;
disabled?: boolean;
/** Skip "already linked" indicator for this task — useful when re-selecting the same task's issue. */
excludeTaskId?: string;
}

export function InlineIssueSelector({
export const InlineIssueSelector = observer(function InlineIssueSelector({
value,
onValueChange,
projectId,
repositoryUrl = '',
projectPath = '',
disabled,
excludeTaskId,
}: InlineIssueSelectorProps) {
const linkedIssueMap = getLinkedIssueMap(projectId, excludeTaskId);
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 New Map instance on every render invalidates useCallback

getLinkedIssueMap returns a freshly constructed Map on every render. Because linkedIssueMap is included in the dependency array of handleKeyDown (line 114), the callback is recreated on every render — negating the memoization benefit. Consider stabilising the reference via useMemo(() => getLinkedIssueMap(projectId, excludeTaskId), [projectId, excludeTaskId]), so the map only rebuilds when the underlying observable data actually changes.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/renderer/features/tasks/components/issue-selector/inline-issue-selector.tsx
Line: 37

Comment:
**New `Map` instance on every render invalidates `useCallback`**

`getLinkedIssueMap` returns a freshly constructed `Map` on every render. Because `linkedIssueMap` is included in the dependency array of `handleKeyDown` (line 114), the callback is recreated on every render — negating the memoization benefit. Consider stabilising the reference via `useMemo(() => getLinkedIssueMap(projectId, excludeTaskId), [projectId, excludeTaskId])`, so the map only rebuilds when the underlying observable data actually changes.

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

const {
issues,
issueProvider,
Expand Down Expand Up @@ -90,7 +96,8 @@ export function InlineIssueSelector({
case 'Enter': {
e.preventDefault();
const issue = issues[highlightedIndex];
if (issue) onValueChange(issue === value ? null : issue);
if (!issue) break;
onValueChange(issue === value ? null : issue);
break;
}
case 'Escape':
Expand Down Expand Up @@ -170,6 +177,7 @@ export function InlineIssueSelector({
issues.map((issue, index) => {
const isSelected = value?.identifier === issue.identifier;
const isHighlighted = index === highlightedIndex;
const linkedTo = linkedIssueMap.get(issue.url);
return (
<button
key={issue.identifier}
Expand All @@ -182,7 +190,7 @@ export function InlineIssueSelector({
onMouseEnter={() => setHighlightedIndex(index)}
onClick={() => onValueChange(isSelected ? null : issue)}
>
<IssueRow issue={issue} />
<IssueRow issue={issue} linkedTo={linkedTo} />
{isSelected && (
<Check className="absolute right-2 size-3.5 shrink-0 text-foreground-muted" />
)}
Expand All @@ -201,4 +209,4 @@ export function InlineIssueSelector({
</div>
</div>
);
}
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ExternalLink, Loader2 } from 'lucide-react';
import { ExternalLink, Link2, Loader2 } from 'lucide-react';
import { observer } from 'mobx-react-lite';
import { forwardRef, useCallback, useRef, useState } from 'react';
import type { Issue } from '@shared/tasks';
import {
Expand All @@ -22,6 +23,7 @@ import {
import { Select, SelectContent, SelectItem, SelectTrigger } from '@renderer/lib/ui/select';
import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/lib/ui/tooltip';
import { cn } from '@renderer/utils/utils';
import { getLinkedIssueMap, type LinkedIssueInfo } from './use-linked-issue-urls';
import { useIssueSearch } from './useIssueSearch';

function getStatusColorClass(status?: string) {
Expand Down Expand Up @@ -75,7 +77,22 @@ export function ProviderLogo({
return <img src={src} alt={alt} className={className ?? 'h-3.5 w-3.5'} />;
}

export function IssueRow({ issue }: { issue: Issue }) {
export function LinkedIssueIndicator({ linkedTo }: { linkedTo: LinkedIssueInfo }) {
return (
<Tooltip>
<TooltipTrigger
render={
<span className="ml-auto flex shrink-0 items-center text-foreground-muted">
<Link2 className="size-3.5" />
</span>
}
/>
<TooltipContent>Already linked to task: {linkedTo.taskName}</TooltipContent>
</Tooltip>
);
}

export function IssueRow({ issue, linkedTo }: { issue: Issue; linkedTo?: LinkedIssueInfo }) {
return (
<span className="flex min-w-0 items-center gap-2 w-full">
<Tooltip>
Expand All @@ -84,6 +101,7 @@ export function IssueRow({ issue }: { issue: Issue }) {
</Tooltip>
<IssueIdentifier identifier={issue.identifier} />
{issue.title ? <span className="truncate text-foreground">{issue.title}</span> : null}
{linkedTo ? <LinkedIssueIndicator linkedTo={linkedTo} /> : null}
</span>
);
}
Expand All @@ -94,15 +112,19 @@ export interface IssueSelectorProps {
projectId?: string;
repositoryUrl: string;
projectPath?: string;
/** Skip "already linked" indicator for this task — useful when re-selecting the same task's issue. */
excludeTaskId?: string;
}

export function IssueSelector({
export const IssueSelector = observer(function IssueSelector({
projectId,
repositoryUrl,
projectPath = '',
value,
onValueChange,
excludeTaskId,
}: IssueSelectorProps) {
const linkedIssueMap = getLinkedIssueMap(projectId, excludeTaskId);
const {
issues,
issueProvider,
Expand Down Expand Up @@ -225,11 +247,14 @@ export function IssueSelector({
<span className="text-muted-foreground">No issues found</span>
</ComboboxEmpty>
<ComboboxList>
{(issue: Issue) => (
<ComboboxItem key={issue.identifier} value={issue} className="pr-2">
<IssueRow issue={issue} />
</ComboboxItem>
)}
{(issue: Issue) => {
const linkedTo = linkedIssueMap.get(issue.url);
return (
<ComboboxItem key={issue.identifier} value={issue} className="pr-2">
<IssueRow issue={issue} linkedTo={linkedTo} />
</ComboboxItem>
);
}}
</ComboboxList>
</ComboboxContent>
</Combobox>
Expand All @@ -238,7 +263,7 @@ export function IssueSelector({
)}
</div>
);
}
});

export function SelectedIssueValue({ issue }: { issue: Issue }) {
return (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { isRegistered } from '@renderer/features/tasks/stores/task';
import { getTaskManagerStore } from '@renderer/features/tasks/stores/task-selectors';

export type LinkedIssueInfo = { taskId: string; taskName: string };

/**
* Reads from observable task state — call only inside `observer` components.
* Returns a map from issue URL → linked task info for non-archived tasks,
* optionally excluding a single task (e.g. when re-selecting the same task's issue).
*/
export function getLinkedIssueMap(
projectId: string | undefined,
excludeTaskId?: string
): Map<string, LinkedIssueInfo> {
const map = new Map<string, LinkedIssueInfo>();
if (!projectId) return map;
const taskManager = getTaskManagerStore(projectId);
if (!taskManager) return map;
for (const store of taskManager.tasks.values()) {
if (!isRegistered(store)) continue;
if (excludeTaskId && store.data.id === excludeTaskId) continue;
if (store.data.archivedAt) continue;
const url = store.data.linkedIssue?.url;
if (!url || map.has(url)) continue;
map.set(url, { taskId: store.data.id, taskName: store.data.name });
}
return map;
}
29 changes: 28 additions & 1 deletion src/renderer/features/tasks/task-titlebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
Terminal,
} from 'lucide-react';
import { observer } from 'mobx-react-lite';
import type { Issue } from '@shared/tasks';
import {
asMounted,
getProjectStore,
Expand All @@ -30,6 +31,7 @@ import { ConnectionStatusDot } from '@renderer/lib/components/connection-status-
import { OpenInMenu } from '@renderer/lib/components/titlebar/open-in-menu';
import { Titlebar } from '@renderer/lib/components/titlebar/Titlebar';
import { useDelayedBoolean } from '@renderer/lib/hooks/use-delay-boolean';
import { rpc } from '@renderer/lib/ipc';
import { Badge } from '@renderer/lib/ui/badge';
import { Button } from '@renderer/lib/ui/button';
import { MicroLabel } from '@renderer/lib/ui/label';
Expand All @@ -39,7 +41,7 @@ import { ToggleGroup, ToggleGroupItem } from '@renderer/lib/ui/toggle-group';
import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/lib/ui/tooltip';
import { cn } from '@renderer/utils/utils';
import { DevServerPills } from './components/dev-server-pills';
import { IssueSelector } from './components/issue-selector/issue-selector';
import { IssueSelector, ProviderLogo } from './components/issue-selector/issue-selector';
import { useTaskViewNavigation } from './hooks/use-task-view-navigation';
import { useTaskViewShortcuts } from './hooks/use-task-view-shortcuts';
import { useGitActions } from './use-git-actions';
Expand Down Expand Up @@ -262,9 +264,11 @@ const ActiveTaskTitlebar = observer(function ActiveTaskTitlebar({
projectId={projectId}
repositoryUrl={provisionedTask.repositoryStore.repositoryUrl ?? ''}
projectPath={provisionedTask.path}
excludeTaskId={taskId}
/>
</PopoverContent>
</Popover>
{taskPayload.linkedIssue ? <LinkedIssueBadge issue={taskPayload.linkedIssue} /> : null}
<button
className={cn(
'text-foreground-muted ml-1',
Expand Down Expand Up @@ -376,3 +380,26 @@ const ActiveTaskTitlebar = observer(function ActiveTaskTitlebar({
/>
);
});

function LinkedIssueBadge({ issue }: { issue: Issue }) {
return (
<Tooltip>
<TooltipTrigger
render={
<button
type="button"
disabled={!issue.url}
onClick={() => {
if (issue.url) void rpc.app.openExternal(issue.url);
}}
className="flex items-center gap-1 rounded-md border border-border px-1.5 py-0.5 text-xs text-foreground-muted hover:bg-muted/30 disabled:cursor-default disabled:opacity-60"
>
<ProviderLogo provider={issue.provider} className="h-3 w-3" />
<span className="font-mono">{issue.identifier}</span>
</button>
}
/>
<TooltipContent>{issue.title || issue.identifier}</TooltipContent>
</Tooltip>
);
}
Loading