Copy startup terminal #5585

webfiltered · 2025-09-15T08:03:48Z

Summary

Adds hover-based copy button to Logs pane, Desktop terminal, and Desktop start view, with smart selection handling.

Changes

Copy button appears on hover at top-right of terminal
Respects existing text selection (copies selection if present, all text otherwise)
Dynamic tooltip shows "Copy selection" or "Copy all" based on state
Proper opacity transitions and hover states
Adds component tests

Screenshots

Top right -->

Desktop server start view

┆Issue is synchronized with this Notion page by Unito

github-actions · 2025-09-15T08:04:09Z

🎭 Playwright Test Results

✅ All tests passed!

⏰ Completed at: 09/15/2025, 11:45:32 PM UTC

📈 Summary

Total Tests: 450
Passed: 421 ✅
Failed: 0
Flaky: 0
Skipped: 29 ⏭️

📊 Test Reports by Browser

✅ chromium: View Report • ✅ 414 / ❌ 0 / ⚠️ 0 / ⏭️ 29
✅ chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
✅ chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
✅ mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

webfiltered · 2025-09-15T08:07:38Z

Note: Based on #5563, which should be merged first

webfiltered · 2025-09-15T09:50:40Z

N.B. This also affects web frontend - in the logs drawer. First screenshot is Chrome. PR says "terminal" mostly because it's xterm running the show. Adding explicitness.

src/components/bottomPanel/tabs/terminal/BaseTerminal.vue

DrJKL · 2025-09-15T19:18:54Z

src/components/bottomPanel/tabs/terminal/BaseTerminal.vue

+emit('created', terminalData, rootEl)
+
+const { terminal } = terminalData
+let selectionDisposable: IDisposable | undefined


I usually see mutable variables like this as a red flag. Should this be a ref?

I always check let usage, because it's usually not what I want. This, afaik, has always been the frontend pattern for this type of value.

Is there any actual benefit to having this be a ref? Seems like it would just add needless overhead for what this is actually used for?

One benefit would be fewer lets 😁

It would also let you more safely reference and reset the value in the lifecycle hooks.

DrJKL · 2025-09-15T19:23:48Z

src/components/bottomPanel/tabs/terminal/BaseTerminal.vue

+})
+
+onUnmounted(() => {
+  selectionDisposable?.dispose()


If you change selectionDisposable to be a ref, you can also use https://vueuse.org/shared/refManualReset/ to prevent double disposals

Unless I'm missing the point, this one feels like attempting to guard against internal Vue failures. My gut reaction is that we should trust the framework to honour its lifecycle. If our framework lifecycle is broken, the app really should just stop executing.

DrJKL · 2025-09-15T19:24:50Z

tests-ui/tests/components/bottomPanel/tabs/terminal/BaseTerminal.spec.ts

+  }))
+}))
+
+const mockTerminal = {


Is there an interface you can declare that this satisfies?

Also, could you move this into the mock as a cached value to prevent direct access?

tests-ui/tests/components/bottomPanel/tabs/terminal/BaseTerminal.spec.ts

github-actions · 2025-09-15T21:36:35Z

🔧 Auto-fixes Applied

This PR has been automatically updated to fix linting and formatting issues.

⚠️ Important: Your local branch is now behind. Run git pull before making additional changes to avoid conflicts.

Changes made:

ESLint auto-fixes
Prettier formatting

DrJKL · 2025-09-15T21:36:29Z

src/components/bottomPanel/tabs/terminal/BaseTerminal.vue

+})
+
+const handleCopy = async () => {
+  const { terminal } = terminalData


Isn't this already present in scope?

src/components/bottomPanel/tabs/terminal/BaseTerminal.vue

…al.spec.ts Co-authored-by: Alexander Brown <[email protected]>

Add assertion to verify onSelectionChange was called

- Current consensus is .test.ts for component files

github-actions · 2025-09-15T23:38:09Z

🔧 Auto-fixes Applied

This PR has been automatically updated to fix linting and formatting issues.

⚠️ Important: Your local branch is now behind. Run git pull before making additional changes to avoid conflicts.

Changes made:

ESLint auto-fixes
Prettier formatting

webfiltered requested review from Yorha4D, KarryCharon, shinshin86 and a team as code owners September 15, 2025 08:03

webfiltered added the Design Used to request Product feedback on design decisions label Sep 15, 2025

dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 15, 2025

comfy-pr-bot requested review from PabloWiedemann and snomiao September 15, 2025 08:13

webfiltered force-pushed the copy-startup-terminal branch from fefb822 to 2ba4060 Compare September 15, 2025 08:15

christian-byrne previously approved these changes Sep 15, 2025

View reviewed changes

christian-byrne added the area:desktop label Sep 15, 2025

DrJKL self-assigned this Sep 15, 2025

DrJKL requested changes Sep 15, 2025

View reviewed changes

DrJKL assigned webfiltered and unassigned DrJKL Sep 15, 2025

webfiltered dismissed christian-byrne’s stale review via 0834e52 September 15, 2025 20:00

PabloWiedemann previously approved these changes Sep 15, 2025

View reviewed changes

webfiltered dismissed PabloWiedemann’s stale review via 67ddf2e September 15, 2025 21:24

DrJKL previously approved these changes Sep 15, 2025

View reviewed changes

webfiltered dismissed DrJKL’s stale review via fe813a7 September 15, 2025 22:08

webfiltered commented Sep 15, 2025

View reviewed changes

src/components/bottomPanel/tabs/terminal/BaseTerminal.vue Outdated Show resolved Hide resolved

webfiltered added 4 commits September 16, 2025 09:25

Add copyTerminal translation key

864d15b

Add copy terminal button with select all functionality

00a1975

Remove copy button from error view button group

896b8c3

Add hover-based copy button overlay to terminal

9557cea

webfiltered and others added 24 commits September 16, 2025 09:25

Replace any type with proper IDisposable type

5816b52

Refactor copy logic for clarity

99abea3

Use v-show for proper opacity transitions

50b1570

Prefer optional chaining

8db7c39

Use useEventListener for context menu

ea82a52

Remove redundant opacity classes

8aa16b9

Add BaseTerminal component tests

2aaf4da

Use pointer-events for button interactivity

33252bd

Update tests for pointer-events button behavior

5fabe65

Fix clipboard mock in tests

58af55b

Fix test expectations for opacity classes

c5658a8

Simplify hover tests for button state

f7bf711

Remove low-value 'renders terminal container' test

c5e51e4

Remove non-functional 'button responds to hover' test

bd5b216

Remove implementation detail test for dispose listener

e1a020e

Remove redundant 'tracks selection changes' test

cb93d65

Remove obvious comments from test file

6c571fd

Use cn() utility for conditional classes

fb0b529

Update tests-ui/tests/components/bottomPanel/tabs/terminal/BaseTermin…

4f4ea64

…al.spec.ts Co-authored-by: Alexander Brown <[email protected]>

[auto-fix] Apply ESLint and Prettier fixes

403fbab

Remove 'any' types from wrapper and terminalMock variables

bd6147d

Add assertion to verify onSelectionChange was called

Move mountBaseTerminal factory to module scope

0948f78

Rename test file

6cb0fe3

- Current consensus is .test.ts for component files

Update src/components/bottomPanel/tabs/terminal/BaseTerminal.vue

c0f07b6

webfiltered force-pushed the copy-startup-terminal branch from 6a4316e to c0f07b6 Compare September 15, 2025 23:25

nit

2c26e83

webfiltered force-pushed the copy-startup-terminal branch from 57a0f23 to 2c26e83 Compare September 15, 2025 23:35

webfiltered merged commit b8d8193 into main Sep 16, 2025
21 checks passed

webfiltered deleted the copy-startup-terminal branch September 16, 2025 00:06

Copy startup terminal #5585

Copy startup terminal #5585

Uh oh!

Conversation

webfiltered commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Screenshots

Uh oh!

github-actions bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎭 Playwright Test Results

📈 Summary

📊 Test Reports by Browser

Uh oh!

webfiltered commented Sep 15, 2025

Uh oh!

webfiltered commented Sep 15, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 15, 2025

🔧 Auto-fixes Applied

Changes made:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Sep 15, 2025

🔧 Auto-fixes Applied

Changes made:

Uh oh!

Uh oh!

Uh oh!

webfiltered commented Sep 15, 2025 •

edited

Loading

github-actions bot commented Sep 15, 2025 •

edited

Loading