Skip to content

fix: delete card confirmation + column Enter key submit (#513 #516) #527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Chris0Jeky merged 4 commits into from
Mar 29, 2026
Merged

fix: delete card confirmation + column Enter key submit (#513 #516) #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf7c562
fix: add confirmation dialog before card deletion (#513)
Chris0Jeky Mar 29, 2026
c382ebe
fix: prevent enter key propagation in column name form (#516)
Chris0Jeky Mar 29, 2026
b0acef0
fix: reset isDeleting in finally block in CardModal (#513)
Chris0Jeky Mar 29, 2026
d335b5a
Fix delete confirmation description rendering " entities
Chris0Jeky Mar 29, 2026
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
51 changes: 48 additions & 3 deletions frontend/taskdeck-web/src/components/board/CardModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { onBeforeUnmount, ref, computed, watch } from 'vue'
import { useBoardStore } from '../../store/boardStore'
import { useSessionStore } from '../../store/sessionStore'
import { useEscapeToClose } from '../../composables/useEscapeToClose'
import TdDialog from '../ui/TdDialog.vue'
import type { Card, CardCaptureProvenance, Label } from '../../types/board'
import type { CardComment } from '../../types/comments'
import { normalizeProposalStatus } from '../../utils/automation'
Expand Down Expand Up @@ -37,6 +38,11 @@ const captureProvenance = ref<CardCaptureProvenance | null>(null)
const captureProvenanceError = ref<string | null>(null)
const loadingCaptureProvenance = ref(false)
const loadedCaptureProvenanceCardId = ref<string | null>(null)
const showDeleteConfirm = ref(false)
const isDeleting = ref(false)
const deleteConfirmDescription = computed(
() => `Are you sure you want to delete "${props.card.title}"? This action cannot be undone.`
)

const comments = computed<CardComment[]>(() => boardStore.getCardComments(props.card.id))
const topLevelComments = computed(() => comments.value.filter(comment => !comment.parentCommentId))
Expand Down Expand Up @@ -158,15 +164,26 @@ async function handleSave() {
}
}

async function handleDelete() {
if (!confirm(`Are you sure you want to delete "${props.card.title}"?`)) return
function handleDeleteClick() {
showDeleteConfirm.value = true
}

function handleDeleteCancel() {
showDeleteConfirm.value = false
}

async function handleDeleteConfirm() {
if (isDeleting.value) return
isDeleting.value = true
try {
await boardStore.deleteCard(props.card.boardId, props.card.id)
showDeleteConfirm.value = false
emit('updated')
emit('close')
} catch (error) {
console.error('Failed to delete card:', error)
} finally {
isDeleting.value = false
}
}

Expand Down Expand Up @@ -593,7 +610,7 @@ onBeforeUnmount(() => {
<!-- Actions -->
<div class="mt-6 flex items-center justify-between">
<button
@click="handleDelete"
@click="handleDeleteClick"
type="button"
class="px-4 py-2 text-sm font-medium text-red-600 hover:text-red-700 hover:bg-red-50 border border-red-300 rounded-md transition-colors"
>
Expand All @@ -620,4 +637,32 @@ onBeforeUnmount(() => {
</div>
</div>
</div>

<!-- Delete Confirmation Dialog -->
<TdDialog
:open="showDeleteConfirm"
title="Delete Card"
:description="deleteConfirmDescription"
:close-on-backdrop="!isDeleting"
@close="handleDeleteCancel"
>
<template #footer>
<button
type="button"
:disabled="isDeleting"
class="px-4 py-2 text-sm font-medium text-gray-700 hover:bg-gray-50 border border-gray-300 rounded-md transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
@click="handleDeleteCancel"
>
Cancel
</button>
<button
type="button"
:disabled="isDeleting"
class="px-4 py-2 text-sm font-medium text-white bg-red-600 hover:bg-red-700 border border-transparent rounded-md transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
@click="handleDeleteConfirm"
>
{{ isDeleting ? 'Deleting…' : 'Delete' }}
</button>
</template>
</TdDialog>
</template>
27 changes: 18 additions & 9 deletions frontend/taskdeck-web/src/tests/components/CardModal.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,50 +290,59 @@ describe('CardModal', () => {
expect((saveButton?.element as HTMLButtonElement).disabled).toBe(true)
})

it('should show delete confirmation before deleting', async () => {
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false)

it('should show delete confirmation dialog before deleting', async () => {
const wrapper = mount(CardModal, {
props: {
card,
isOpen: true,
labels,
},
attachTo: document.body,
})

const deleteButton = wrapper
.findAll('button')
.find((btn) => btn.text().includes('Delete Card'))
await deleteButton?.trigger('click')
await wrapper.vm.$nextTick()

expect(confirmSpy).toHaveBeenCalled()
// Dialog should now be open; deleteCard must NOT have been called yet
expect(mockStore.deleteCard).not.toHaveBeenCalled()
// The confirmation dialog should be visible in the DOM
expect(document.querySelector('.td-dialog')).not.toBeNull()

confirmSpy.mockRestore()
wrapper.unmount()
})

it('should call deleteCard when deletion is confirmed', async () => {
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true)

it('should call deleteCard when deletion is confirmed in dialog', async () => {
const wrapper = mount(CardModal, {
props: {
card,
isOpen: true,
labels,
},
attachTo: document.body,
})

// Open the confirmation dialog
const deleteButton = wrapper
.findAll('button')
.find((btn) => btn.text().includes('Delete Card'))
await deleteButton?.trigger('click')
await wrapper.vm.$nextTick()

// Click the "Delete" confirm button inside the dialog
const confirmButton = Array.from(document.querySelectorAll<HTMLButtonElement>('button')).find(
(btn) => btn.textContent?.trim() === 'Delete',
)
Comment on lines +335 to +337
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 29, 2026

Choose a reason for hiding this comment

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

medium

The current implementation to find the confirmation button queries the entire document, which could lead to flaky tests if another button with the text 'Delete' is present. It's better to scope the query to the dialog element to make the test more robust. It would also be good practice to assert that dialogElement is not null before using it.

Suggested change
const confirmButton = Array.from(document.querySelectorAll<HTMLButtonElement>('button')).find(
(btn) => btn.textContent?.trim() === 'Delete',
)
const dialogElement = document.querySelector('.td-dialog');
const confirmButton = Array.from(dialogElement!.querySelectorAll<HTMLButtonElement>('button')).find(
(btn) => btn.textContent?.trim() === 'Delete',
);

confirmButton?.click()
await wrapper.vm.$nextTick()
await wrapper.vm.$nextTick()

expect(mockStore.deleteCard).toHaveBeenCalledWith('board-1', 'card-1')
expect(wrapper.emitted('close')).toBeTruthy()

confirmSpy.mockRestore()
wrapper.unmount()
})

it('should handle label selection', async () => {
Expand Down
1 change: 1 addition & 0 deletions frontend/taskdeck-web/src/views/BoardView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ useKeyboardShortcuts([
placeholder="Column name"
class="td-column-form__input"
autofocus
@keydown.enter.prevent="createColumn"
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 29, 2026

Choose a reason for hiding this comment

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

medium

The @keydown.enter.prevent handler on this input is redundant. The parent <form> element already has a @submit.prevent="createColumn" handler. Standard browser behavior will trigger the form's submit event when Enter is pressed in a text input, so the existing form handler is sufficient. Removing this duplicate handler simplifies the code and relies on idiomatic form handling.

/>
<button
type="submit"
Expand Down
Loading