Skip to content

Commit 45ca1be

Browse files
MyesteryMyestery
authored
fix: address small CodeRabbit issues (#9229)
## Summary Address several small CodeRabbit-filed issues: clipboard simplification, queue getter cleanup, pointer handling, and test parameterization. ## Changes - **What**: - Simplify `useCopyToClipboard` by using VueUse's built-in `legacy` mode instead of a manual `document.execCommand` fallback - Remove `queueIndex` getter alias from `TaskItemImpl`, replace all usages with `job.priority` - Add `pointercancel` event handling and try-catch around `releasePointerCapture` in `useNodeResize` to prevent stuck resize state - Parameterize repetitive `getNodeProvider` tests in `modelToNodeStore.test.ts` using `it.each()` - Fixes #9024 - Fixes #7955 - Fixes #7323 - Fixes #8703 ## Review Focus - `useCopyToClipboard`: VueUse's `legacy: true` enables the `execCommand` fallback internally — verify browser compat is acceptable - `useNodeResize`: cleanup logic extracted into shared function used by both `pointerup` and `pointercancel`
1 parent aef299c commit 45ca1be

File tree

9 files changed

+123
-198
lines changed

9 files changed

+123
-198
lines changed

‎src/components/queue/job/JobDetailsPopover.stories.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,11 @@ export const Queued: Story = {
131131
const exec = useExecutionStore()
132132

133133
const jobId = 'job-queued-1'
134-
const queueIndex = 104
134+
const priority = 104
135135

136136
// Current job in pending
137137
queue.pendingTasks = [
138-
makePendingTask(jobId, queueIndex, Date.now() - 90_000)
138+
makePendingTask(jobId, priority, Date.now() - 90_000)
139139
]
140140
// Add some other pending jobs to give context
141141
queue.pendingTasks.push(
@@ -179,13 +179,13 @@ export const QueuedParallel: Story = {
179179
const exec = useExecutionStore()
180180

181181
const jobId = 'job-queued-parallel'
182-
const queueIndex = 210
182+
const priority = 210
183183

184184
// Current job in pending with some ahead
185185
queue.pendingTasks = [
186186
makePendingTask('job-ahead-1', 200, Date.now() - 180_000),
187187
makePendingTask('job-ahead-2', 205, Date.now() - 150_000),
188-
makePendingTask(jobId, queueIndex, Date.now() - 120_000)
188+
makePendingTask(jobId, priority, Date.now() - 120_000)
189189
]
190190

191191
// Seen 2 minutes ago - set via prompt metadata above
@@ -238,9 +238,9 @@ export const Running: Story = {
238238
const exec = useExecutionStore()
239239

240240
const jobId = 'job-running-1'
241-
const queueIndex = 300
241+
const priority = 300
242242
queue.runningTasks = [
243-
makeRunningTask(jobId, queueIndex, Date.now() - 65_000)
243+
makeRunningTask(jobId, priority, Date.now() - 65_000)
244244
]
245245
queue.historyTasks = [
246246
makeHistoryTask('hist-r1', 250, 30, true),
@@ -279,10 +279,10 @@ export const QueuedZeroAheadSingleRunning: Story = {
279279
const exec = useExecutionStore()
280280

281281
const jobId = 'job-queued-zero-ahead-single'
282-
const queueIndex = 510
282+
const priority = 510
283283

284284
queue.pendingTasks = [
285-
makePendingTask(jobId, queueIndex, Date.now() - 45_000)
285+
makePendingTask(jobId, priority, Date.now() - 45_000)
286286
]
287287

288288
queue.historyTasks = [
@@ -324,10 +324,10 @@ export const QueuedZeroAheadMultiRunning: Story = {
324324
const exec = useExecutionStore()
325325

326326
const jobId = 'job-queued-zero-ahead-multi'
327-
const queueIndex = 520
327+
const priority = 520
328328

329329
queue.pendingTasks = [
330-
makePendingTask(jobId, queueIndex, Date.now() - 20_000)
330+
makePendingTask(jobId, priority, Date.now() - 20_000)
331331
]
332332

333333
queue.historyTasks = [
@@ -380,8 +380,8 @@ export const Completed: Story = {
380380
const queue = useQueueStore()
381381

382382
const jobId = 'job-completed-1'
383-
const queueIndex = 400
384-
queue.historyTasks = [makeHistoryTask(jobId, queueIndex, 37, true)]
383+
const priority = 400
384+
queue.historyTasks = [makeHistoryTask(jobId, priority, 37, true)]
385385

386386
return { args: { ...args, jobId } }
387387
},
@@ -401,11 +401,11 @@ export const Failed: Story = {
401401
const queue = useQueueStore()
402402

403403
const jobId = 'job-failed-1'
404-
const queueIndex = 410
404+
const priority = 410
405405
queue.historyTasks = [
406406
makeHistoryTask(
407407
jobId,
408-
queueIndex,
408+
priority,
409409
12,
410410
false,
411411
'Example error: invalid inputs for node X'

‎src/components/queue/job/JobDetailsPopover.vue‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,14 +168,14 @@ const queuedAtValue = computed(() =>
168168
169169
const currentQueueIndex = computed<number | null>(() => {
170170
const task = taskForJob.value
171-
return task ? Number(task.queueIndex) : null
171+
return task ? Number(task.job.priority) : null
172172
})
173173
174174
const jobsAhead = computed<number | null>(() => {
175175
const idx = currentQueueIndex.value
176176
if (idx == null) return null
177177
const ahead = queueStore.pendingTasks.filter(
178-
(t: TaskItemImpl) => Number(t.queueIndex) < idx
178+
(t: TaskItemImpl) => Number(t.job.priority) < idx
179179
)
180180
return ahead.length
181181
})

‎src/composables/queue/useJobList.test.ts‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type { TaskItemImpl } from '@/stores/queueStore'
1111

1212
type TestTask = {
1313
jobId: string
14-
queueIndex: number
14+
job: { priority: number }
1515
mockState: JobState
1616
executionTime?: number
1717
executionEndTimestamp?: number
@@ -174,7 +174,7 @@ const createTask = (
174174
overrides: Partial<TestTask> & { mockState?: JobState } = {}
175175
): TestTask => ({
176176
jobId: overrides.jobId ?? `task-${Math.random().toString(36).slice(2, 7)}`,
177-
queueIndex: overrides.queueIndex ?? 0,
177+
job: overrides.job ?? { priority: 0 },
178178
mockState: overrides.mockState ?? 'pending',
179179
executionTime: overrides.executionTime,
180180
executionEndTimestamp: overrides.executionEndTimestamp,
@@ -258,7 +258,7 @@ describe('useJobList', () => {
258258
it('tracks recently added pending jobs and clears the hint after expiry', async () => {
259259
vi.useFakeTimers()
260260
queueStoreMock.pendingTasks = [
261-
createTask({ jobId: '1', queueIndex: 1, mockState: 'pending' })
261+
createTask({ jobId: '1', job: { priority: 1 }, mockState: 'pending' })
262262
]
263263

264264
const { jobItems } = initComposable()
@@ -287,7 +287,7 @@ describe('useJobList', () => {
287287
vi.useFakeTimers()
288288
const taskId = '2'
289289
queueStoreMock.pendingTasks = [
290-
createTask({ jobId: taskId, queueIndex: 1, mockState: 'pending' })
290+
createTask({ jobId: taskId, job: { priority: 1 }, mockState: 'pending' })
291291
]
292292

293293
const { jobItems } = initComposable()
@@ -300,7 +300,7 @@ describe('useJobList', () => {
300300

301301
vi.mocked(buildJobDisplay).mockClear()
302302
queueStoreMock.pendingTasks = [
303-
createTask({ jobId: taskId, queueIndex: 2, mockState: 'pending' })
303+
createTask({ jobId: taskId, job: { priority: 2 }, mockState: 'pending' })
304304
]
305305
await flush()
306306
jobItems.value
@@ -314,7 +314,7 @@ describe('useJobList', () => {
314314
it('cleans up timeouts on unmount', async () => {
315315
vi.useFakeTimers()
316316
queueStoreMock.pendingTasks = [
317-
createTask({ jobId: '3', queueIndex: 1, mockState: 'pending' })
317+
createTask({ jobId: '3', job: { priority: 1 }, mockState: 'pending' })
318318
]
319319

320320
initComposable()
@@ -331,23 +331,23 @@ describe('useJobList', () => {
331331
queueStoreMock.pendingTasks = [
332332
createTask({
333333
jobId: 'p',
334-
queueIndex: 1,
334+
job: { priority: 1 },
335335
mockState: 'pending',
336336
createTime: 3000
337337
})
338338
]
339339
queueStoreMock.runningTasks = [
340340
createTask({
341341
jobId: 'r',
342-
queueIndex: 5,
342+
job: { priority: 5 },
343343
mockState: 'running',
344344
createTime: 2000
345345
})
346346
]
347347
queueStoreMock.historyTasks = [
348348
createTask({
349349
jobId: 'h',
350-
queueIndex: 3,
350+
job: { priority: 3 },
351351
mockState: 'completed',
352352
createTime: 1000,
353353
executionEndTimestamp: 5000
@@ -366,9 +366,9 @@ describe('useJobList', () => {
366366

367367
it('filters by job tab and resets failed tab when failures disappear', async () => {
368368
queueStoreMock.historyTasks = [
369-
createTask({ jobId: 'c', queueIndex: 3, mockState: 'completed' }),
370-
createTask({ jobId: 'f', queueIndex: 2, mockState: 'failed' }),
371-
createTask({ jobId: 'p', queueIndex: 1, mockState: 'pending' })
369+
createTask({ jobId: 'c', job: { priority: 3 }, mockState: 'completed' }),
370+
createTask({ jobId: 'f', job: { priority: 2 }, mockState: 'failed' }),
371+
createTask({ jobId: 'p', job: { priority: 1 }, mockState: 'pending' })
372372
]
373373

374374
const instance = initComposable()
@@ -384,7 +384,7 @@ describe('useJobList', () => {
384384
expect(instance.hasFailedJobs.value).toBe(true)
385385

386386
queueStoreMock.historyTasks = [
387-
createTask({ jobId: 'c', queueIndex: 3, mockState: 'completed' })
387+
createTask({ jobId: 'c', job: { priority: 3 }, mockState: 'completed' })
388388
]
389389
await flush()
390390

@@ -396,13 +396,13 @@ describe('useJobList', () => {
396396
queueStoreMock.pendingTasks = [
397397
createTask({
398398
jobId: 'wf-1',
399-
queueIndex: 2,
399+
job: { priority: 2 },
400400
mockState: 'pending',
401401
workflowId: 'workflow-1'
402402
}),
403403
createTask({
404404
jobId: 'wf-2',
405-
queueIndex: 1,
405+
job: { priority: 1 },
406406
mockState: 'pending',
407407
workflowId: 'workflow-2'
408408
})
@@ -426,14 +426,14 @@ describe('useJobList', () => {
426426
queueStoreMock.historyTasks = [
427427
createTask({
428428
jobId: 'alpha',
429-
queueIndex: 2,
429+
job: { priority: 2 },
430430
mockState: 'completed',
431431
createTime: 2000,
432432
executionEndTimestamp: 2000
433433
}),
434434
createTask({
435435
jobId: 'beta',
436-
queueIndex: 1,
436+
job: { priority: 1 },
437437
mockState: 'failed',
438438
createTime: 1000,
439439
executionEndTimestamp: 1000
@@ -471,13 +471,13 @@ describe('useJobList', () => {
471471
queueStoreMock.runningTasks = [
472472
createTask({
473473
jobId: 'active',
474-
queueIndex: 3,
474+
job: { priority: 3 },
475475
mockState: 'running',
476476
executionTime: 7_200_000
477477
}),
478478
createTask({
479479
jobId: 'other',
480-
queueIndex: 2,
480+
job: { priority: 2 },
481481
mockState: 'running',
482482
executionTime: 3_600_000
483483
})
@@ -507,7 +507,7 @@ describe('useJobList', () => {
507507
queueStoreMock.runningTasks = [
508508
createTask({
509509
jobId: 'live-preview',
510-
queueIndex: 1,
510+
job: { priority: 1 },
511511
mockState: 'running'
512512
})
513513
]
@@ -526,7 +526,7 @@ describe('useJobList', () => {
526526
queueStoreMock.runningTasks = [
527527
createTask({
528528
jobId: 'disabled-preview',
529-
queueIndex: 1,
529+
job: { priority: 1 },
530530
mockState: 'running'
531531
})
532532
]
@@ -567,28 +567,28 @@ describe('useJobList', () => {
567567
queueStoreMock.historyTasks = [
568568
createTask({
569569
jobId: 'today-small',
570-
queueIndex: 4,
570+
job: { priority: 4 },
571571
mockState: 'completed',
572572
executionEndTimestamp: Date.now(),
573573
executionTime: 2_000
574574
}),
575575
createTask({
576576
jobId: 'today-large',
577-
queueIndex: 3,
577+
job: { priority: 3 },
578578
mockState: 'completed',
579579
executionEndTimestamp: Date.now(),
580580
executionTime: 5_000
581581
}),
582582
createTask({
583583
jobId: 'yesterday',
584-
queueIndex: 2,
584+
job: { priority: 2 },
585585
mockState: 'failed',
586586
executionEndTimestamp: Date.now() - 86_400_000,
587587
executionTime: 1_000
588588
}),
589589
createTask({
590590
jobId: 'undated',
591-
queueIndex: 1,
591+
job: { priority: 1 },
592592
mockState: 'pending'
593593
})
594594
]

‎src/composables/useCopyToClipboard.ts‎

Lines changed: 19 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -4,64 +4,32 @@ import { useToast } from 'primevue/usetoast'
44
import { t } from '@/i18n'
55

66
export function useCopyToClipboard() {
7-
const { copy, copied } = useClipboard()
7+
const { copy, copied } = useClipboard({ legacy: true })
88
const toast = useToast()
9-
const showSuccessToast = () => {
10-
toast.add({
11-
severity: 'success',
12-
summary: t('g.success'),
13-
detail: t('clipboard.successMessage'),
14-
life: 3000
15-
})
16-
}
17-
const showErrorToast = () => {
18-
toast.add({
19-
severity: 'error',
20-
summary: t('g.error'),
21-
detail: t('clipboard.errorMessage')
22-
})
23-
}
24-
25-
function fallbackCopy(text: string) {
26-
const textarea = document.createElement('textarea')
27-
textarea.setAttribute('readonly', '')
28-
textarea.value = text
29-
textarea.style.position = 'absolute'
30-
textarea.style.left = '-9999px'
31-
textarea.setAttribute('aria-hidden', 'true')
32-
textarea.setAttribute('tabindex', '-1')
33-
textarea.style.width = '1px'
34-
textarea.style.height = '1px'
35-
document.body.appendChild(textarea)
36-
textarea.select()
37-
38-
try {
39-
// using legacy document.execCommand for fallback for old and linux browsers
40-
const successful = document.execCommand('copy')
41-
if (successful) {
42-
showSuccessToast()
43-
} else {
44-
showErrorToast()
45-
}
46-
} catch (err) {
47-
showErrorToast()
48-
} finally {
49-
textarea.remove()
50-
}
51-
}
529

53-
const copyToClipboard = async (text: string) => {
10+
async function copyToClipboard(text: string) {
5411
try {
5512
await copy(text)
5613
if (copied.value) {
57-
showSuccessToast()
14+
toast.add({
15+
severity: 'success',
16+
summary: t('g.success'),
17+
detail: t('clipboard.successMessage'),
18+
life: 3000
19+
})
5820
} else {
59-
// If VueUse copy failed, try fallback
60-
fallbackCopy(text)
21+
toast.add({
22+
severity: 'error',
23+
summary: t('g.error'),
24+
detail: t('clipboard.errorMessage')
25+
})
6126
}
62-
} catch (err) {
63-
// VueUse copy failed, try fallback
64-
fallbackCopy(text)
27+
} catch {
28+
toast.add({
29+
severity: 'error',
30+
summary: t('g.error'),
31+
detail: t('clipboard.errorMessage')
32+
})
6533
}
6634
}
6735

0 commit comments

Comments
 (0)