Skip to content

Commit 66e9fe2

Browse files
committed
fix: do not try to open a dialog when one is closing
This created a bug when clicking on "Add a filter for this field", which is calling "dialog.accept()" and then opening a new dialog. In this case, the new dialog opens before the "close" event has been sent, so it catches it wrongly.
1 parent 7315cc4 commit 66e9fe2

File tree

2 files changed

+78
-34
lines changed

2 files changed

+78
-34
lines changed

umap/static/umap/js/modules/ui/dialog.js

Lines changed: 29 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ export default class Dialog extends WithTemplate {
106106
}
107107

108108
open(settings = {}) {
109-
this.dialog.returnValue = undefined
110109
const dialog = Object.assign({}, this.settings, settings)
111110
this.dialog.className = 'umap-dialog window'
112111
if (dialog.className) {
@@ -118,7 +117,6 @@ export default class Dialog extends WithTemplate {
118117
this.elements.cancel.hidden = !dialog.cancel
119118
this.elements.message.textContent = dialog.message
120119
this.elements.message.hidden = !dialog.message
121-
this.elements.target = dialog.target || ''
122120
this.elements.template.innerHTML = ''
123121
if (dialog.template?.nodeType === 1) {
124122
this.elements.template.appendChild(dialog.template)
@@ -137,12 +135,13 @@ export default class Dialog extends WithTemplate {
137135
if (currentZIndex) {
138136
this.dialog.style.zIndex = currentZIndex + 1
139137
}
140-
141-
this.toggle(true)
142-
138+
if (this.dialogSupported) {
139+
this.dialog.show()
140+
} else {
141+
this.dialog.hidden = false
142+
}
143143
if (this.hasFormData) this.focusable[0].focus()
144144
else this.elements.accept.focus()
145-
146145
return this.waitForUser()
147146
}
148147

@@ -151,44 +150,40 @@ export default class Dialog extends WithTemplate {
151150
}
152151

153152
close() {
154-
this.toggle(false)
153+
this._closing = true
154+
if (this.dialogSupported) {
155+
this.dialog.close()
156+
} else {
157+
this.dialog.hidden = true
158+
this.dialog.dispatchEvent(new CustomEvent('close'))
159+
}
155160
}
156161

157162
accept() {
158163
this.dialog.returnValue = 'accept'
159164
this.close()
160165
}
161166

162-
toggle(open = false) {
163-
if (this.dialogSupported) {
164-
if (open) {
165-
this.dialog.show()
166-
} else {
167-
this.dialog.close()
167+
waitForUser() {
168+
return new Promise((resolve) => {
169+
const onClose = () => {
170+
this._closing = false
171+
if (this.dialog.returnValue === 'accept') {
172+
const value = this.hasFormData ? this.collectFormData() : true
173+
resolve(value)
174+
}
168175
}
169-
} else {
170-
this.dialog.hidden = !open
171-
if (this.elements.target && !open) {
172-
this.elements.target.focus()
176+
const waitForClose = () => {
177+
this.dialog.returnValue = undefined
178+
this.dialog.addEventListener('close', () => onClose(), { once: true })
173179
}
174-
if (!open) {
175-
this.dialog.dispatchEvent(new CustomEvent('close'))
180+
if (this._closing) {
181+
// We are opening a new dialog while another is not fully closed,
182+
// so let's first wait for that one to be fully closed
183+
this.dialog.addEventListener('close', () => waitForClose(), { once: true })
184+
} else {
185+
waitForClose()
176186
}
177-
}
178-
}
179-
180-
waitForUser() {
181-
return new Promise((resolve) => {
182-
this.dialog.addEventListener(
183-
'close',
184-
(event) => {
185-
if (this.dialog.returnValue === 'accept') {
186-
const value = this.hasFormData ? this.collectFormData() : true
187-
resolve(value)
188-
}
189-
},
190-
{ once: true }
191-
)
192187
})
193188
}
194189

umap/tests/integration/test_filters.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,3 +419,52 @@ def test_deleting_field_from_map_should_delete_filter(live_server, page, openmap
419419
"foobar": {"widget": "minmax", "label": "Foo Bar"},
420420
"name": {"widget": "checkbox", "label": "Bar Foo"},
421421
}
422+
423+
424+
def test_can_create_filter_from_new_field(live_server, page, openmap):
425+
openmap.settings["properties"]["onLoadPanel"] = "datafilters"
426+
openmap.save()
427+
DataLayerFactory(map=openmap, data=DATALAYER_DATA1)
428+
page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit")
429+
page.get_by_role("button", name="Manage layers").click()
430+
page.get_by_role("button", name="Edit", exact=True).nth(1).click()
431+
page.get_by_role("heading", name="Fields, filters and keys").click()
432+
page.get_by_role("button", name="Add a new field").click()
433+
page.get_by_role("textbox", name="Field Name ✔").fill("foobar")
434+
page.get_by_role("button", name="Add filter for this field").click()
435+
page.get_by_role("textbox", name="Human readable name of the").fill("Foo Bar")
436+
page.wait_for_timeout(300) # Input throttling.
437+
page.get_by_text("Edit this field").click()
438+
expect(page.locator(".umap-filter span").filter(has_text="Foo Bar")).to_be_visible()
439+
with page.expect_response(re.compile(r".*/datalayer/update/")):
440+
page.get_by_role("button", name="Save").click()
441+
saved = DataLayer.objects.first()
442+
assert saved.settings["fields"] == [
443+
{
444+
"key": "mytype",
445+
"type": "String",
446+
},
447+
{
448+
"key": "name",
449+
"type": "String",
450+
},
451+
{
452+
"key": "mynumber",
453+
"type": "String",
454+
},
455+
{
456+
"key": "mydate",
457+
"type": "String",
458+
},
459+
{
460+
"key": "foobar",
461+
"type": "String",
462+
},
463+
]
464+
465+
assert saved.settings["filters"] == {
466+
"foobar": {
467+
"label": "Foo Bar",
468+
"widget": "checkbox",
469+
},
470+
}

0 commit comments

Comments
 (0)