Skip to content

Commit b240c09

Browse files
authored
Fix: Escape closes Settings dialog if login dialog open (#4364)
1 parent 309a5b8 commit b240c09

File tree

3 files changed

+97
-6
lines changed

3 files changed

+97
-6
lines changed

src/services/dialogService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ export const useDialogService = () => {
272272
onSuccess: () => resolve(true)
273273
},
274274
dialogComponentProps: {
275-
closable: false,
275+
closable: true,
276276
onClose: () => resolve(false)
277277
}
278278
})

src/stores/dialogStore.ts

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ interface CustomDialogComponentProps {
2626
modal?: boolean
2727
position?: DialogPosition
2828
pt?: DialogPassThroughOptions
29+
closeOnEscape?: boolean
30+
dismissableMask?: boolean
2931
}
3032

3133
type DialogComponentProps = InstanceType<typeof GlobalDialog>['$props'] &
@@ -62,6 +64,12 @@ export interface ShowDialogOptions {
6264
export const useDialogStore = defineStore('dialog', () => {
6365
const dialogStack = ref<DialogInstance[]>([])
6466

67+
/**
68+
* The key of the currently active (top-most) dialog.
69+
* Only the active dialog can be closed with the ESC key.
70+
*/
71+
const activeKey = ref<string | null>(null)
72+
6573
const genDialogKey = () => `dialog-${Math.random().toString(36).slice(2, 9)}`
6674

6775
/**
@@ -87,17 +95,27 @@ export const useDialogStore = defineStore('dialog', () => {
8795
if (index !== -1) {
8896
const [dialog] = dialogStack.value.splice(index, 1)
8997
insertDialogByPriority(dialog)
98+
activeKey.value = dialogKey
99+
updateCloseOnEscapeStates()
90100
}
91101
}
92102

93103
function closeDialog(options?: { key: string }) {
94104
const targetDialog = options
95105
? dialogStack.value.find((d) => d.key === options.key)
96-
: dialogStack.value[0]
106+
: dialogStack.value.find((d) => d.key === activeKey.value)
97107
if (!targetDialog) return
98108

99109
targetDialog.dialogComponentProps?.onClose?.()
100-
dialogStack.value.splice(dialogStack.value.indexOf(targetDialog), 1)
110+
const index = dialogStack.value.indexOf(targetDialog)
111+
dialogStack.value.splice(index, 1)
112+
113+
activeKey.value =
114+
dialogStack.value.length > 0
115+
? dialogStack.value[dialogStack.value.length - 1].key
116+
: null
117+
118+
updateCloseOnEscapeStates()
101119
}
102120

103121
function createDialog(options: {
@@ -114,7 +132,7 @@ export const useDialogStore = defineStore('dialog', () => {
114132
dialogStack.value.shift()
115133
}
116134

117-
const dialog: DialogInstance = {
135+
const dialog = {
118136
key: options.key,
119137
visible: true,
120138
title: options.title,
@@ -135,7 +153,6 @@ export const useDialogStore = defineStore('dialog', () => {
135153
dismissableMask: true,
136154
...options.dialogComponentProps,
137155
maximized: false,
138-
// @ts-expect-error TODO: fix this
139156
onMaximize: () => {
140157
dialog.dialogComponentProps.maximized = true
141158
},
@@ -156,10 +173,29 @@ export const useDialogStore = defineStore('dialog', () => {
156173
}
157174

158175
insertDialogByPriority(dialog)
176+
activeKey.value = options.key
177+
updateCloseOnEscapeStates()
159178

160179
return dialog
161180
}
162181

182+
/**
183+
* Ensures only the top-most dialog in the stack can be closed with the Escape key.
184+
* This is necessary because PrimeVue Dialogs do not handle `closeOnEscape` prop
185+
* correctly when multiple dialogs are open.
186+
*/
187+
function updateCloseOnEscapeStates() {
188+
const topDialog = dialogStack.value.find((d) => d.key === activeKey.value)
189+
const topClosable = topDialog?.dialogComponentProps.closable
190+
191+
dialogStack.value.forEach((dialog) => {
192+
dialog.dialogComponentProps = {
193+
...dialog.dialogComponentProps,
194+
closeOnEscape: dialog === topDialog && !!topClosable
195+
}
196+
})
197+
}
198+
163199
function showDialog(options: ShowDialogOptions) {
164200
const dialogKey = options.key || genDialogKey()
165201

@@ -206,6 +242,7 @@ export const useDialogStore = defineStore('dialog', () => {
206242
showDialog,
207243
closeDialog,
208244
showExtensionDialog,
209-
isDialogOpen
245+
isDialogOpen,
246+
activeKey
210247
}
211248
})

tests-ui/tests/store/dialogStore.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,4 +172,58 @@ describe('dialogStore', () => {
172172
expect(store.dialogStack[0].title).toBe('Original Title')
173173
})
174174
})
175+
176+
describe('ESC key behavior with multiple dialogs', () => {
177+
it('should only allow the active dialog to close with ESC key', () => {
178+
const store = useDialogStore()
179+
180+
// Create dialogs with different priorities
181+
store.showDialog({
182+
key: 'dialog-1',
183+
component: MockComponent,
184+
priority: 1
185+
})
186+
187+
store.showDialog({
188+
key: 'dialog-2',
189+
component: MockComponent,
190+
priority: 2
191+
})
192+
193+
store.showDialog({
194+
key: 'dialog-3',
195+
component: MockComponent,
196+
priority: 3
197+
})
198+
199+
// Only the active dialog should be closable with ESC
200+
const activeDialog = store.dialogStack.find(
201+
(d) => d.key === store.activeKey
202+
)
203+
const inactiveDialogs = store.dialogStack.filter(
204+
(d) => d.key !== store.activeKey
205+
)
206+
207+
expect(activeDialog?.dialogComponentProps.closeOnEscape).toBe(true)
208+
inactiveDialogs.forEach((dialog) => {
209+
expect(dialog.dialogComponentProps.closeOnEscape).toBe(false)
210+
})
211+
212+
// Close the active dialog
213+
store.closeDialog({ key: store.activeKey! })
214+
215+
// The new active dialog should now be closable with ESC
216+
const newActiveDialog = store.dialogStack.find(
217+
(d) => d.key === store.activeKey
218+
)
219+
const newInactiveDialogs = store.dialogStack.filter(
220+
(d) => d.key !== store.activeKey
221+
)
222+
223+
expect(newActiveDialog?.dialogComponentProps.closeOnEscape).toBe(true)
224+
newInactiveDialogs.forEach((dialog) => {
225+
expect(dialog.dialogComponentProps.closeOnEscape).toBe(false)
226+
})
227+
})
228+
})
175229
})

0 commit comments

Comments
 (0)