Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 20 additions & 2 deletions frontend/taskdeck-web/src/components/board/CardModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ const props = withDefaults(defineProps<{
isOpen: boolean
labels: Label[]
presentation?: 'modal' | 'inspector'
suppressDiscardPrompt?: boolean
skipFocusRestore?: boolean
}>(), {
presentation: 'modal',
suppressDiscardPrompt: false,
skipFocusRestore: false,
})

const emit = defineEmits<{
Expand Down Expand Up @@ -113,7 +117,11 @@ watch(
await nextTick()
focusInitialControl()
} else if (wasOpen) {
restoreFocus()
if (props.skipFocusRestore) {
previouslyFocusedElement = null
} else {
restoreFocus()
}
}
},
{ immediate: true },
Expand Down Expand Up @@ -142,7 +150,7 @@ watch(
)

onUnmounted(() => {
if (props.isOpen) {
if (props.isOpen && !props.skipFocusRestore) {
restoreFocus()
}
})
Expand Down Expand Up @@ -234,7 +242,16 @@ watch(hasUnsavedChanges, (dirty) => {
emit('dirty-change', dirty)
}, { immediate: true })

watch(() => props.suppressDiscardPrompt, (suppress) => {
if (!suppress) return

pendingThinkingPath.value = null
showDiscardConfirm.value = false
}, { immediate: true })

function handleClose() {
if (props.suppressDiscardPrompt) return

if (hasUnsavedChanges.value) {
showDiscardConfirm.value = true
return
Expand All @@ -245,6 +262,7 @@ function handleClose() {
useEscapeToClose(
() =>
props.isOpen &&
!props.suppressDiscardPrompt &&
!showDiscardConfirm.value &&
!showDeleteConfirm.value &&
!showCommentDeleteConfirm.value,
Expand Down
24 changes: 24 additions & 0 deletions frontend/taskdeck-web/src/tests/components/CardModal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,30 @@ describe('CardModal', () => {
wrapper.unmount()
})

it('hands discard confirmation to the parent without reopening a stale local prompt', async () => {
const wrapper = mount(CardModal, {
props: { card, isOpen: true, labels, presentation: 'inspector' },
attachTo: document.body,
})
await nextTick()

await wrapper.get('#card-title').setValue('Unsaved title')
await wrapper.get('[aria-label="Close card editor"]').trigger('click')
await nextTick()
expect(document.body.querySelector('[data-testid="card-discard-confirm"]')).not.toBeNull()

await wrapper.setProps({ suppressDiscardPrompt: true })
await nextTick()
expect(document.body.querySelector('[data-testid="card-discard-confirm"]')).toBeNull()

await wrapper.setProps({ suppressDiscardPrompt: false })
await nextTick()
expect(document.body.querySelector('[data-testid="card-discard-confirm"]')).toBeNull()
expect(wrapper.emitted('close')).toBeUndefined()

wrapper.unmount()
})

it('should not render when isOpen is false', () => {
const wrapper = mount(CardModal, {
props: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ function mountView(props: Record<string, unknown> = {}) {
stubs: {
CardModal: {
name: 'CardModal',
props: ['card', 'isOpen', 'labels', 'presentation'],
props: ['card', 'isOpen', 'labels', 'presentation', 'suppressDiscardPrompt', 'skipFocusRestore'],
emits: ['dirty-change', 'updated', 'close'],
template: '<div v-if="isOpen" data-testid="paper-card-modal" :data-presentation="presentation">{{ card.title }}</div>',
},
Expand Down Expand Up @@ -369,6 +369,30 @@ describe('PaperBoardView', () => {
expect(wrapper.find('[data-testid="paper-card-modal"]').exists()).toBe(false)
})

it('lets the parent own the single confirmation when route navigation supersedes card close', async () => {
const wrapper = mountView()
await openDirtyCard(wrapper, cardsByColumn.get('col-backlog')![0]!)
const modal = wrapper.findComponent({ name: 'CardModal' })

const cancelledNavigation = routeLeaveGuard!()
await nextTick()
expect(modal.props('suppressDiscardPrompt')).toBe(true)
expect(wrapper.findAll('[role="dialog"]')).toHaveLength(1)

await wrapper.get('[data-testid="card-switch-cancel"]').trigger('click')
await expect(cancelledNavigation).resolves.toBe(false)
await nextTick()
expect(modal.props('suppressDiscardPrompt')).toBe(false)
expect(wrapper.get('[data-testid="paper-card-modal"]').text()).toContain('A')

const confirmedNavigation = routeUpdateGuard!()
await nextTick()
expect(modal.props('suppressDiscardPrompt')).toBe(true)
await wrapper.get('[data-testid="card-switch-confirm"]').trigger('click')
await expect(confirmedNavigation).resolves.toBe(true)
expect(wrapper.find('[data-testid="paper-card-modal"]').exists()).toBe(false)
})

it('guards reused board-route changes while the inspector is dirty', async () => {
const wrapper = mountView()
wrapper.findAllComponents(PaperBoardColumn)[0]!.vm.$emit(
Expand Down
15 changes: 13 additions & 2 deletions frontend/taskdeck-web/src/views/paper/PaperBoardView.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script setup lang="ts">
import { computed, onBeforeUnmount, onMounted, ref, watch } from 'vue'
import { computed, nextTick, onBeforeUnmount, onMounted, ref, watch } from 'vue'
import { useI18n } from 'vue-i18n'
import { onBeforeRouteLeave, onBeforeRouteUpdate, useRoute, useRouter } from 'vue-router'
import { useBoardStore } from '../../store/boardStore'
Expand Down Expand Up @@ -101,6 +101,7 @@ const selectedCard = ref<Card | null>(null)
const pendingCard = ref<Card | null>(null)
const pendingNavigation = ref<{ resolve: (allow: boolean) => void } | null>(null)
const cardEditorDirty = ref(false)
const routeDiscarding = ref(false)
type BoardDensity = 'comfortable' | 'compact'
const BOARD_DENSITY_KEY = 'td.paper.board-density.v1'
const density = ref<BoardDensity>('comfortable')
Expand Down Expand Up @@ -341,6 +342,7 @@ const activeSelectedCardId = computed(() => props.selectedCardId ?? selectedCard
watch(boardId, () => {
selectedCard.value = null
pendingCard.value = null
routeDiscarding.value = false
cardEditorDirty.value = false
// Switching boards must not carry a half-typed card draft, an open column
// dialog, or an error banner across to a board they do not belong to.
Expand Down Expand Up @@ -438,6 +440,8 @@ function onLaneDragStart(column: Column, event: DragEvent) {
}

function openCard(card: Card) {
routeDiscarding.value = false

if (selectedCard.value?.id === card.id) return
if (selectedCard.value && cardEditorDirty.value) {
pendingCard.value = card
Expand All @@ -447,6 +451,7 @@ function openCard(card: Card) {
}

function closeCard() {
routeDiscarding.value = false
pendingNavigation.value?.resolve(false)
pendingNavigation.value = null
selectedCard.value = null
Expand All @@ -470,6 +475,7 @@ function closeCard() {
* other two exits from this dialog use.
*/
function handleCardUpdated() {
routeDiscarding.value = false
const navigation = pendingNavigation.value
pendingNavigation.value = null
cardEditorDirty.value = false
Expand All @@ -481,12 +487,13 @@ function handleCardEditorDirtyChange(dirty: boolean) {
}

function cancelPendingDiscard() {
routeDiscarding.value = false
pendingCard.value = null
pendingNavigation.value?.resolve(false)
pendingNavigation.value = null
}

function confirmPendingDiscard() {
async function confirmPendingDiscard() {
const cardToOpen = pendingCard.value
const navigation = pendingNavigation.value
pendingCard.value = null
Expand All @@ -497,6 +504,8 @@ function confirmPendingDiscard() {
return
}
if (navigation) {
routeDiscarding.value = true
await nextTick()
selectedCard.value = null
navigation.resolve(true)
}
Expand Down Expand Up @@ -1127,6 +1136,8 @@ async function addStarterColumns() {
:is-open="Boolean(selectedCard)"
:labels="boardStore.currentBoardLabels"
:presentation="cardPresentation"
:suppress-discard-prompt="discardDialogOpen"
:skip-focus-restore="routeDiscarding"
@close="closeCard"
@updated="handleCardUpdated"
@dirty-change="handleCardEditorDirtyChange"
Expand Down
92 changes: 92 additions & 0 deletions frontend/taskdeck-web/tests/e2e/board-discard-coordination.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { expect, test, type APIRequestContext, type Page } from '@playwright/test'
import { API_BASE_URL, registerAndAttachSession, type AuthResult } from './support/authSession'
import { createBoardWithColumn } from './support/boardHelpers'
import { assertOk } from './support/httpAsserts'

async function addCard(
request: APIRequestContext,
auth: AuthResult,
boardId: string,
title: string,
) {
const columnsResponse = await request.get(`${API_BASE_URL}/boards/${boardId}/columns`, {
headers: { Authorization: `Bearer ${auth.token}` },
})
await assertOk(columnsResponse, 'List discard coordination board columns')
const columns = await columnsResponse.json() as Array<{ id: string }>
const response = await request.post(`${API_BASE_URL}/boards/${boardId}/cards`, {
headers: { Authorization: `Bearer ${auth.token}` },
data: { title, description: '', columnId: columns[0]!.id, position: 0 },
})
await assertOk(response, `Create discard coordination card '${title}'`)
}

async function openRouteDiscardPrompt(page: Page) {
await page.goBack()
await expect(page.getByTestId('card-switch-confirm')).toBeVisible()
await expect(page.getByRole('dialog', { name: 'Discard card changes?' })).toHaveCount(1)
await expect(page.getByTestId('card-discard-confirm')).toHaveCount(0)
}

test.describe('Paper board discard coordination', () => {
test('keeps one confirmation owner across card close and browser Back', async ({ page, request }) => {
const auth = await registerAndAttachSession(page, request, 'discard-coordination')
const seed = `${Date.now()}-${Math.floor(Math.random() * 1_000_000)}`
const boardId = await createBoardWithColumn(request, auth, seed, {
boardNamePrefix: 'Discard coordination',
description: 'Browser discard coordination regression',
columnNamePrefix: 'Backlog',
})
const boardName = `Discard coordination ${seed}`
const cardTitle = `Unsaved discard ${seed}`
await addCard(request, auth, boardId, cardTitle)

await page.goto('/workspace/boards')
await expect(page.getByRole('button', { name: '+ New Board' })).toBeVisible()
const boardCard = page.locator('.paper-boards__card').filter({ hasText: boardName })
await expect(boardCard).toBeVisible()
await boardCard.click()
await expect(page).toHaveURL(new RegExp(`/workspace/boards/${boardId}$`))
await expect(page.locator('[data-testid="paper-board-lanes"]')).toBeVisible()

const cardOpener = page.getByRole('button', { name: `Card ${cardTitle}`, exact: true })
await cardOpener.click()
const editor = page.getByRole('dialog', { name: 'Edit Card' })
await expect(editor).toBeVisible()
await page.locator('#card-title').fill('Unsaved title')

await page.getByRole('button', { name: 'Close card editor' }).click()
await expect(page.getByTestId('card-discard-confirm')).toBeVisible()
await expect(page.getByRole('dialog', { name: 'Discard card changes?' })).toHaveCount(1)

await openRouteDiscardPrompt(page)
await expect(page.getByRole('dialog', { name: 'Discard card changes?' })).toBeFocused()
await page.keyboard.press('Escape')
await expect(page).toHaveURL(new RegExp(`/workspace/boards/${boardId}$`))
await expect(page.locator('#card-title')).toHaveValue('Unsaved title')
await expect(page.getByTestId('card-discard-confirm')).toHaveCount(0)
await expect(page.getByRole('button', { name: 'Close card editor' })).toBeFocused()

await page.getByRole('button', { name: 'Close card editor' }).click()
await expect(page.getByTestId('card-discard-confirm')).toBeVisible()
await openRouteDiscardPrompt(page)
await page.getByTestId('card-switch-confirm').click()

await expect(page).toHaveURL('/workspace/boards')
await expect(page.getByRole('button', { name: '+ New Board' })).toBeVisible()
await expect(page.getByRole('button', { name: 'Close card editor' })).toHaveCount(0)
await expect(page.getByTestId('card-discard-confirm')).toHaveCount(0)
const focusAfterNavigation = await page.evaluate(() => {
const active = document.activeElement
return {
isConnected: active?.isConnected ?? false,
isOnBoardsDestination: active === document.body || (active instanceof Element && active.closest('.paper-boards') !== null),
testId: active?.getAttribute('data-testid'),
}
})
expect(focusAfterNavigation.isConnected).toBe(true)
expect(focusAfterNavigation.isOnBoardsDestination).toBe(true)
expect(focusAfterNavigation.testId).not.toBe('card-switch-confirm')
expect(focusAfterNavigation.testId).not.toBe('card-discard-confirm')
})
})