Fix: Escape closes Settings dialog if login dialog open (#4364)

This commit is contained in:
Sidharth
2025-07-24 11:11:26 +05:30
committed by GitHub
parent 309a5b8c9a
commit b240c090aa
3 changed files with 97 additions and 6 deletions

View File

@@ -272,7 +272,7 @@ export const useDialogService = () => {
onSuccess: () => resolve(true)
},
dialogComponentProps: {
closable: false,
closable: true,
onClose: () => resolve(false)
}
})

View File

@@ -26,6 +26,8 @@ interface CustomDialogComponentProps {
modal?: boolean
position?: DialogPosition
pt?: DialogPassThroughOptions
closeOnEscape?: boolean
dismissableMask?: boolean
}
type DialogComponentProps = InstanceType<typeof GlobalDialog>['$props'] &
@@ -62,6 +64,12 @@ export interface ShowDialogOptions {
export const useDialogStore = defineStore('dialog', () => {
const dialogStack = ref<DialogInstance[]>([])
/**
* The key of the currently active (top-most) dialog.
* Only the active dialog can be closed with the ESC key.
*/
const activeKey = ref<string | null>(null)
const genDialogKey = () => `dialog-${Math.random().toString(36).slice(2, 9)}`
/**
@@ -87,17 +95,27 @@ export const useDialogStore = defineStore('dialog', () => {
if (index !== -1) {
const [dialog] = dialogStack.value.splice(index, 1)
insertDialogByPriority(dialog)
activeKey.value = dialogKey
updateCloseOnEscapeStates()
}
}
function closeDialog(options?: { key: string }) {
const targetDialog = options
? dialogStack.value.find((d) => d.key === options.key)
: dialogStack.value[0]
: dialogStack.value.find((d) => d.key === activeKey.value)
if (!targetDialog) return
targetDialog.dialogComponentProps?.onClose?.()
dialogStack.value.splice(dialogStack.value.indexOf(targetDialog), 1)
const index = dialogStack.value.indexOf(targetDialog)
dialogStack.value.splice(index, 1)
activeKey.value =
dialogStack.value.length > 0
? dialogStack.value[dialogStack.value.length - 1].key
: null
updateCloseOnEscapeStates()
}
function createDialog(options: {
@@ -114,7 +132,7 @@ export const useDialogStore = defineStore('dialog', () => {
dialogStack.value.shift()
}
const dialog: DialogInstance = {
const dialog = {
key: options.key,
visible: true,
title: options.title,
@@ -135,7 +153,6 @@ export const useDialogStore = defineStore('dialog', () => {
dismissableMask: true,
...options.dialogComponentProps,
maximized: false,
// @ts-expect-error TODO: fix this
onMaximize: () => {
dialog.dialogComponentProps.maximized = true
},
@@ -156,10 +173,29 @@ export const useDialogStore = defineStore('dialog', () => {
}
insertDialogByPriority(dialog)
activeKey.value = options.key
updateCloseOnEscapeStates()
return dialog
}
/**
* Ensures only the top-most dialog in the stack can be closed with the Escape key.
* This is necessary because PrimeVue Dialogs do not handle `closeOnEscape` prop
* correctly when multiple dialogs are open.
*/
function updateCloseOnEscapeStates() {
const topDialog = dialogStack.value.find((d) => d.key === activeKey.value)
const topClosable = topDialog?.dialogComponentProps.closable
dialogStack.value.forEach((dialog) => {
dialog.dialogComponentProps = {
...dialog.dialogComponentProps,
closeOnEscape: dialog === topDialog && !!topClosable
}
})
}
function showDialog(options: ShowDialogOptions) {
const dialogKey = options.key || genDialogKey()
@@ -206,6 +242,7 @@ export const useDialogStore = defineStore('dialog', () => {
showDialog,
closeDialog,
showExtensionDialog,
isDialogOpen
isDialogOpen,
activeKey
}
})

View File

@@ -172,4 +172,58 @@ describe('dialogStore', () => {
expect(store.dialogStack[0].title).toBe('Original Title')
})
})
describe('ESC key behavior with multiple dialogs', () => {
it('should only allow the active dialog to close with ESC key', () => {
const store = useDialogStore()
// Create dialogs with different priorities
store.showDialog({
key: 'dialog-1',
component: MockComponent,
priority: 1
})
store.showDialog({
key: 'dialog-2',
component: MockComponent,
priority: 2
})
store.showDialog({
key: 'dialog-3',
component: MockComponent,
priority: 3
})
// Only the active dialog should be closable with ESC
const activeDialog = store.dialogStack.find(
(d) => d.key === store.activeKey
)
const inactiveDialogs = store.dialogStack.filter(
(d) => d.key !== store.activeKey
)
expect(activeDialog?.dialogComponentProps.closeOnEscape).toBe(true)
inactiveDialogs.forEach((dialog) => {
expect(dialog.dialogComponentProps.closeOnEscape).toBe(false)
})
// Close the active dialog
store.closeDialog({ key: store.activeKey! })
// The new active dialog should now be closable with ESC
const newActiveDialog = store.dialogStack.find(
(d) => d.key === store.activeKey
)
const newInactiveDialogs = store.dialogStack.filter(
(d) => d.key !== store.activeKey
)
expect(newActiveDialog?.dialogComponentProps.closeOnEscape).toBe(true)
newInactiveDialogs.forEach((dialog) => {
expect(dialog.dialogComponentProps.closeOnEscape).toBe(false)
})
})
})
})