Skip to content

Commit f0f90e9

Browse files
committed
Address Copilot PR feedback
1 parent f5961b8 commit f0f90e9

2 files changed

Lines changed: 32 additions & 25 deletions

File tree

src/renderer/App.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,9 @@ export function App() {
168168
const removeExitListener = window.api.onTerminalExit(({ id }) => {
169169
const state = useAppStore.getState()
170170
if (consumePendingTerminalClose(id)) {
171-
state.removeTerminal(id)
171+
if (state.terminals.has(id)) {
172+
state.removeTerminal(id)
173+
}
172174
const assignedTask = (state.config?.tasks || []).find(
173175
(t) => t.assignedSessionId === id && t.status === 'in_progress'
174176
)

src/renderer/components/ConfirmPopover.tsx

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,18 @@ interface ConfirmPopoverProps {
1616
destructive?: boolean
1717
}
1818

19+
function readAnchorRect(anchorEl: HTMLElement): AnchorRect {
20+
const rect = anchorEl.getBoundingClientRect()
21+
return {
22+
top: rect.top,
23+
left: rect.left,
24+
right: rect.right,
25+
bottom: rect.bottom,
26+
width: rect.width,
27+
height: rect.height
28+
}
29+
}
30+
1931
export function ConfirmPopover({
2032
children,
2133
message,
@@ -24,14 +36,14 @@ export function ConfirmPopover({
2436
destructive = true
2537
}: ConfirmPopoverProps) {
2638
const [open, setOpen] = useState(false)
27-
const [anchorRect, setAnchorRect] = useState<AnchorRect | null>(null)
2839
const [position, setPosition] = useState({ top: 0, left: 0, placement: 'bottom' as const })
40+
const anchorRef = useRef<HTMLSpanElement | null>(null)
2941
const popoverRef = useRef<HTMLDivElement>(null)
3042

31-
const updatePosition = (nextAnchor: AnchorRect): void => {
43+
const updatePosition = (anchorEl: HTMLElement): void => {
3244
const popoverRect = popoverRef.current?.getBoundingClientRect()
3345
const { top, left, placement } = calculatePopoverPosition(
34-
nextAnchor,
46+
readAnchorRect(anchorEl),
3547
{
3648
width: popoverRect?.width ?? 220,
3749
height: popoverRect?.height ?? 86
@@ -44,23 +56,12 @@ export function ConfirmPopover({
4456
setPosition({ top, left, placement })
4557
}
4658

47-
const handleTrigger = (e: React.MouseEvent) => {
59+
const handleTrigger = (e: React.MouseEvent<HTMLSpanElement>) => {
4860
e.stopPropagation()
4961
e.preventDefault()
50-
// The wrapper div uses display:contents so getBoundingClientRect returns zeros.
51-
// Measure the actual clicked element (button) instead.
52-
const el = (e.target as HTMLElement).closest('button') ?? (e.target as HTMLElement)
53-
const rect = el.getBoundingClientRect()
54-
const nextAnchor = {
55-
top: rect.top,
56-
left: rect.left,
57-
right: rect.right,
58-
bottom: rect.bottom,
59-
width: rect.width,
60-
height: rect.height
61-
}
62-
setAnchorRect(nextAnchor)
63-
updatePosition(nextAnchor)
62+
const anchorEl = anchorRef.current ?? e.currentTarget
63+
anchorRef.current = anchorEl
64+
updatePosition(anchorEl)
6465
setOpen(true)
6566
}
6667

@@ -91,13 +92,17 @@ export function ConfirmPopover({
9192
}, [open])
9293

9394
useLayoutEffect(() => {
94-
if (!open || !anchorRect) return
95+
if (!open || !anchorRef.current) return
9596

96-
let frame = window.requestAnimationFrame(() => updatePosition(anchorRect))
97+
let frame = window.requestAnimationFrame(() => {
98+
if (anchorRef.current) updatePosition(anchorRef.current)
99+
})
97100

98101
const handleViewportChange = () => {
99102
window.cancelAnimationFrame(frame)
100-
frame = window.requestAnimationFrame(() => updatePosition(anchorRect))
103+
frame = window.requestAnimationFrame(() => {
104+
if (anchorRef.current) updatePosition(anchorRef.current)
105+
})
101106
}
102107
window.addEventListener('resize', handleViewportChange)
103108
window.addEventListener('scroll', handleViewportChange, true)
@@ -107,15 +112,15 @@ export function ConfirmPopover({
107112
window.removeEventListener('resize', handleViewportChange)
108113
window.removeEventListener('scroll', handleViewportChange, true)
109114
}
110-
}, [open, anchorRect])
115+
}, [open])
111116

112117
const motionOffset = position.placement === 'top' ? 4 : -4
113118

114119
return (
115120
<>
116-
<div onClick={handleTrigger} className="contents">
121+
<span ref={anchorRef} onClick={handleTrigger} className="inline-flex">
117122
{children}
118-
</div>
123+
</span>
119124

120125
{createPortal(
121126
<AnimatePresence>

0 commit comments

Comments
 (0)