Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
864d15b
Add copyTerminal translation key
webfiltered Sep 14, 2025
00a1975
Add copy terminal button with select all functionality
webfiltered Sep 14, 2025
896b8c3
Remove copy button from error view button group
webfiltered Sep 14, 2025
9557cea
Add hover-based copy button overlay to terminal
webfiltered Sep 14, 2025
fa143b3
Fix clipboard copy implementation in BaseTerminal
webfiltered Sep 14, 2025
791ebc3
Add 'Copy all' tooltip to terminal copy button
webfiltered Sep 14, 2025
3cd1e42
Fix copy button to be away from right hand side
webfiltered Sep 14, 2025
94c3b94
Update copy button to respect existing selection
webfiltered Sep 14, 2025
d59619c
Add dynamic tooltip showing actual copy action
webfiltered Sep 14, 2025
9e1b7b8
Remove redundant i18n
webfiltered Sep 14, 2025
473b8cc
Fix aria-label to use dynamic tooltip text
webfiltered Sep 14, 2025
761adf1
Remove debug console.error statements from useTerminal
webfiltered Sep 14, 2025
502aa25
Remove redundant keyboard handling from useTerminal
webfiltered Sep 14, 2025
3ef375a
Use Tailwind transition classes instead of custom CSS
webfiltered Sep 14, 2025
b00b18c
Use VueUse useElementHover for robust hover handling
webfiltered Sep 14, 2025
5d0ce42
Move tooltip to left of button
webfiltered Sep 14, 2025
44dcd9f
Simplify code
webfiltered Sep 14, 2025
2e190dc
Fix listener lifecycle management
webfiltered Sep 15, 2025
5816b52
Replace any type with proper IDisposable type
webfiltered Sep 15, 2025
99abea3
Refactor copy logic for clarity
webfiltered Sep 15, 2025
50b1570
Use v-show for proper opacity transitions
webfiltered Sep 15, 2025
8db7c39
Prefer optional chaining
webfiltered Sep 15, 2025
ea82a52
Use useEventListener for context menu
webfiltered Sep 15, 2025
8aa16b9
Remove redundant opacity classes
webfiltered Sep 15, 2025
2aaf4da
Add BaseTerminal component tests
webfiltered Sep 15, 2025
33252bd
Use pointer-events for button interactivity
webfiltered Sep 15, 2025
5fabe65
Update tests for pointer-events button behavior
webfiltered Sep 15, 2025
58af55b
Fix clipboard mock in tests
webfiltered Sep 15, 2025
c5658a8
Fix test expectations for opacity classes
webfiltered Sep 15, 2025
f7bf711
Simplify hover tests for button state
webfiltered Sep 15, 2025
c5e51e4
Remove low-value 'renders terminal container' test
webfiltered Sep 15, 2025
bd5b216
Remove non-functional 'button responds to hover' test
webfiltered Sep 15, 2025
e1a020e
Remove implementation detail test for dispose listener
webfiltered Sep 15, 2025
cb93d65
Remove redundant 'tracks selection changes' test
webfiltered Sep 15, 2025
6c571fd
Remove obvious comments from test file
webfiltered Sep 15, 2025
fb0b529
Use cn() utility for conditional classes
webfiltered Sep 15, 2025
4f4ea64
Update tests-ui/tests/components/bottomPanel/tabs/terminal/BaseTermin…
webfiltered Sep 15, 2025
403fbab
[auto-fix] Apply ESLint and Prettier fixes
actions-user Sep 15, 2025
bd6147d
Remove 'any' types from wrapper and terminalMock variables
webfiltered Sep 15, 2025
0948f78
Move mountBaseTerminal factory to module scope
webfiltered Sep 15, 2025
6cb0fe3
Rename test file
webfiltered Sep 15, 2025
c0f07b6
Update src/components/bottomPanel/tabs/terminal/BaseTerminal.vue
webfiltered Sep 15, 2025
2c26e83
nit
webfiltered Sep 15, 2025
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
71 changes: 67 additions & 4 deletions src/components/bottomPanel/tabs/terminal/BaseTerminal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,77 @@
<div class="p-terminal rounded-none h-full w-full p-2">
<div ref="terminalEl" class="h-full terminal-host" />
</div>
<Button
v-tooltip.left="{
value: tooltipText,
showDelay: 300
}"
icon="pi pi-copy"
severity="secondary"
size="small"
:class="
cn('absolute top-2 right-8 transition-opacity', {
'opacity-0 pointer-events-none select-none': !isHovered
})
"
:aria-label="tooltipText"
@click="handleCopy"
/>
</div>
</template>

<script setup lang="ts">
import { useEventListener } from '@vueuse/core'
import { Ref, onUnmounted, ref } from 'vue'
import { useElementHover, useEventListener } from '@vueuse/core'
import type { IDisposable } from '@xterm/xterm'
import Button from 'primevue/button'
import { Ref, computed, onMounted, onUnmounted, ref } from 'vue'
import { useI18n } from 'vue-i18n'

import { useTerminal } from '@/composables/bottomPanelTabs/useTerminal'
import { electronAPI, isElectron } from '@/utils/envUtil'
import { cn } from '@/utils/tailwindUtil'

const { t } = useI18n()

const emit = defineEmits<{
created: [ReturnType<typeof useTerminal>, Ref<HTMLElement | undefined>]
unmounted: []
}>()
const terminalEl = ref<HTMLElement | undefined>()
const rootEl = ref<HTMLElement | undefined>()
emit('created', useTerminal(terminalEl), rootEl)
const hasSelection = ref(false)

const isHovered = useElementHover(rootEl)

const terminalData = useTerminal(terminalEl)
emit('created', terminalData, rootEl)

const { terminal } = terminalData
let selectionDisposable: IDisposable | undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One benefit would be fewer lets 😁

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


const tooltipText = computed(() => {
return hasSelection.value
? t('serverStart.copySelectionTooltip')
: t('serverStart.copyAllTooltip')
})

const handleCopy = async () => {
const existingSelection = terminal.getSelection()
const shouldSelectAll = !existingSelection
if (shouldSelectAll) terminal.selectAll()

const selectedText = shouldSelectAll
? terminal.getSelection()
: existingSelection

if (selectedText) {
await navigator.clipboard.writeText(selectedText)

if (shouldSelectAll) {
terminal.clearSelection()
}
}
}

const showContextMenu = (event: MouseEvent) => {
event.preventDefault()
Expand All @@ -30,7 +84,16 @@ if (isElectron()) {
useEventListener(terminalEl, 'contextmenu', showContextMenu)
}

onUnmounted(() => emit('unmounted'))
onMounted(() => {
selectionDisposable = terminal.onSelectionChange(() => {
hasSelection.value = terminal.hasSelection()
})
})

onUnmounted(() => {
selectionDisposable?.dispose()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

emit('unmounted')
})
</script>

<style scoped>
Expand Down
2 changes: 2 additions & 0 deletions src/locales/en/main.json
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,8 @@
"reportIssue": "Report Issue",
"openLogs": "Open Logs",
"showTerminal": "Show Terminal",
"copySelectionTooltip": "Copy selection",
"copyAllTooltip": "Copy all",
"process": {
"initial-state": "Loading...",
"python-setup": "Setting up Python Environment...",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
import { createTestingPinia } from '@pinia/testing'
import { VueWrapper, mount } from '@vue/test-utils'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { nextTick } from 'vue'
import { createI18n } from 'vue-i18n'

import BaseTerminal from '@/components/bottomPanel/tabs/terminal/BaseTerminal.vue'

// Mock xterm and related modules
vi.mock('@xterm/xterm', () => ({
Terminal: vi.fn().mockImplementation(() => ({
open: vi.fn(),
dispose: vi.fn(),
onSelectionChange: vi.fn(() => {
// Return a disposable
return {
dispose: vi.fn()
}
}),
hasSelection: vi.fn(() => false),
getSelection: vi.fn(() => ''),
selectAll: vi.fn(),
clearSelection: vi.fn(),
loadAddon: vi.fn()
})),
IDisposable: vi.fn()
}))

vi.mock('@xterm/addon-fit', () => ({
FitAddon: vi.fn().mockImplementation(() => ({
fit: vi.fn(),
proposeDimensions: vi.fn(() => ({ rows: 24, cols: 80 }))
}))
}))

const mockTerminal = {
open: vi.fn(),
dispose: vi.fn(),
onSelectionChange: vi.fn(() => ({
dispose: vi.fn()
})),
hasSelection: vi.fn(() => false),
getSelection: vi.fn(() => ''),
selectAll: vi.fn(),
clearSelection: vi.fn()
}

vi.mock('@/composables/bottomPanelTabs/useTerminal', () => ({
useTerminal: vi.fn(() => ({
terminal: mockTerminal,
useAutoSize: vi.fn(() => ({ resize: vi.fn() }))
}))
}))

vi.mock('@/utils/envUtil', () => ({
isElectron: vi.fn(() => false),
electronAPI: vi.fn(() => null)
}))

// Mock clipboard API
Object.defineProperty(navigator, 'clipboard', {
value: {
writeText: vi.fn().mockResolvedValue(undefined)
},
configurable: true
})

const i18n = createI18n({
legacy: false,
locale: 'en',
messages: {
en: {
serverStart: {
copySelectionTooltip: 'Copy selection',
copyAllTooltip: 'Copy all'
}
}
}
})

const mountBaseTerminal = () => {
return mount(BaseTerminal, {
global: {
plugins: [
createTestingPinia({
createSpy: vi.fn
}),
i18n
],
stubs: {
Button: {
template: '<button v-bind="$attrs"><slot /></button>',
props: ['icon', 'severity', 'size']
}
}
}
})
}

describe('BaseTerminal', () => {
let wrapper: VueWrapper<InstanceType<typeof BaseTerminal>> | undefined

beforeEach(() => {
vi.clearAllMocks()
})

afterEach(() => {
wrapper?.unmount()
})

it('emits created event on mount', () => {
wrapper = mountBaseTerminal()

expect(wrapper.emitted('created')).toBeTruthy()
expect(wrapper.emitted('created')![0]).toHaveLength(2)
})

it('emits unmounted event on unmount', () => {
wrapper = mountBaseTerminal()
wrapper.unmount()

expect(wrapper.emitted('unmounted')).toBeTruthy()
})

it('button exists and has correct initial state', async () => {
wrapper = mountBaseTerminal()

const button = wrapper.find('button[aria-label]')
expect(button.exists()).toBe(true)

expect(button.classes()).toContain('opacity-0')
expect(button.classes()).toContain('pointer-events-none')
})

it('shows correct tooltip when no selection', async () => {
mockTerminal.hasSelection.mockReturnValue(false)
wrapper = mountBaseTerminal()

await wrapper.trigger('mouseenter')
await nextTick()

const button = wrapper.find('button[aria-label]')
expect(button.attributes('aria-label')).toBe('Copy all')
})

it('shows correct tooltip when selection exists', async () => {
mockTerminal.hasSelection.mockReturnValue(true)
wrapper = mountBaseTerminal()

// Trigger the selection change callback that was registered during mount
expect(mockTerminal.onSelectionChange).toHaveBeenCalled()
// Access the mock calls - TypeScript can't infer the mock structure dynamically
const selectionCallback = (mockTerminal.onSelectionChange as any).mock
.calls[0][0]
selectionCallback()
await nextTick()

await wrapper.trigger('mouseenter')
await nextTick()

const button = wrapper.find('button[aria-label]')
expect(button.attributes('aria-label')).toBe('Copy selection')
})

it('copies selected text when selection exists', async () => {
const selectedText = 'selected text'
mockTerminal.hasSelection.mockReturnValue(true)
mockTerminal.getSelection.mockReturnValue(selectedText)

wrapper = mountBaseTerminal()

await wrapper.trigger('mouseenter')
await nextTick()

const button = wrapper.find('button[aria-label]')
await button.trigger('click')

expect(mockTerminal.selectAll).not.toHaveBeenCalled()
expect(navigator.clipboard.writeText).toHaveBeenCalledWith(selectedText)
expect(mockTerminal.clearSelection).not.toHaveBeenCalled()
})

it('copies all text when no selection exists', async () => {
const allText = 'all terminal content'
mockTerminal.hasSelection.mockReturnValue(false)
mockTerminal.getSelection
.mockReturnValueOnce('') // First call returns empty (no selection)
.mockReturnValueOnce(allText) // Second call after selectAll returns all text

wrapper = mountBaseTerminal()

await wrapper.trigger('mouseenter')
await nextTick()

const button = wrapper.find('button[aria-label]')
await button.trigger('click')

expect(mockTerminal.selectAll).toHaveBeenCalled()
expect(navigator.clipboard.writeText).toHaveBeenCalledWith(allText)
expect(mockTerminal.clearSelection).toHaveBeenCalled()
})

it('does not copy when no text available', async () => {
mockTerminal.hasSelection.mockReturnValue(false)
mockTerminal.getSelection.mockReturnValue('')

wrapper = mountBaseTerminal()

await wrapper.trigger('mouseenter')
await nextTick()

const button = wrapper.find('button[aria-label]')
await button.trigger('click')

expect(mockTerminal.selectAll).toHaveBeenCalled()
expect(navigator.clipboard.writeText).not.toHaveBeenCalled()
})
})