mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-20 00:02:19 +00:00
fix(drafts): OtherUsersDraftsModal — close on Fork, don't leak clicks through nested JSON
Two bugs in the per-editor "another user has a draft" banner:
- Fork landed the immediate save but didn't close the banner before
navigating. Svelte hadn't torn down the previous route's components
by the time goto returned, so the banner lingered on top of the
destination editor. Comment the explicit isOpen=false on the
happy path so it's clear it MUST run before goto.
- Clicking anywhere on the screen while the View JSON drilldown was
open closed the underlying banner too. Modal2's clickOutside
action fired on every Modal2 instance — both the JSON modal and
the underlying banner — because both attach their own listener at
the document level. Add `closeOnOutsideClick` opt-out on Modal2
and pass `closeOnOutsideClick={!jsonOpen}` to the outer modal so
clicks outside the JSON drilldown only close the drilldown.
Drive-by: Modal2's keydown handler now ignores Escape when its own
isOpen is false (was a no-op closer that would still preventDefault
on every key press, swallowing key events for any siblings).
This commit is contained in:
@@ -61,7 +61,7 @@
|
||||
<AlertTriangle size={20} class="text-yellow-500 shrink-0 mt-0.5" />
|
||||
<div class="text-sm text-secondary flex flex-col gap-1">
|
||||
<p>
|
||||
Someone else (another tab, browser, or teammate) saved a newer version of this draft. Your
|
||||
Someone else (another tab, browser or AI Agent) saved a newer version of this draft. Your
|
||||
autosave was rejected to avoid overwriting their work.
|
||||
</p>
|
||||
{#if conflictHandle.conflict}
|
||||
|
||||
@@ -97,7 +97,7 @@
|
||||
// Bypass the autosave debouncer so the fork lands on the
|
||||
// server BEFORE we navigate. The destination route loads
|
||||
// via `getDraft=true` and 404s if no draft yet exists at
|
||||
// the fork path — the prior `UserDraft.save` call
|
||||
// the fork path — `UserDraft.save` alone would have
|
||||
// scheduled a debounced POST 1.5s out, so a fresh nav was
|
||||
// always too early.
|
||||
await UserDraftDbSyncer.save({
|
||||
@@ -107,6 +107,12 @@
|
||||
value,
|
||||
immediate: true
|
||||
})
|
||||
// Close the banner BEFORE the navigation so the user sees the
|
||||
// modal disappear on click. Without this the modal stays
|
||||
// visible during the navigation tear-down — Svelte hasn't
|
||||
// torn down the previous route's components by the time
|
||||
// `goto` returns, so the banner lingers on top of the
|
||||
// destination editor for a beat.
|
||||
isOpen = false
|
||||
goto(editPathFor(target))
|
||||
} catch (e) {
|
||||
@@ -122,6 +128,7 @@
|
||||
title="Other users are currently working on {path}"
|
||||
fixedWidth="sm"
|
||||
fixedHeight="sm"
|
||||
closeOnOutsideClick={!jsonOpen}
|
||||
>
|
||||
<div class="flex flex-col w-full gap-4">
|
||||
<div class="flex gap-3 items-start">
|
||||
|
||||
@@ -17,6 +17,11 @@
|
||||
fixedWidth?: 'xs' | 'sm' | 'md' | 'lg' | 'xl' | 'xxl'
|
||||
fixedHeight?: 'xs' | 'sm' | 'md' | 'lg' | 'xl' | 'xxl'
|
||||
contentClasses?: string
|
||||
/** Close when the user clicks outside the modal body. Default
|
||||
* true. Set false when the caller stacks a child modal on top
|
||||
* and clicks "outside" the child would otherwise propagate
|
||||
* here and close the underlying modal. */
|
||||
closeOnOutsideClick?: boolean
|
||||
headerLeft?: import('svelte').Snippet
|
||||
headerRight?: import('svelte').Snippet
|
||||
children?: import('svelte').Snippet
|
||||
@@ -33,6 +38,7 @@
|
||||
fixedWidth = 'md',
|
||||
fixedHeight = 'md',
|
||||
contentClasses = '',
|
||||
closeOnOutsideClick = true,
|
||||
headerLeft,
|
||||
headerRight,
|
||||
children
|
||||
@@ -64,6 +70,7 @@
|
||||
}
|
||||
|
||||
function handleKeyDown(event: KeyboardEvent) {
|
||||
if (!isOpen) return
|
||||
if (event.key === 'Escape') {
|
||||
event.preventDefault()
|
||||
event.stopPropagation()
|
||||
@@ -94,7 +101,9 @@
|
||||
css?.popup?.class,
|
||||
'wm-modal-form-popup'
|
||||
)}
|
||||
use:clickOutside={{ onClickOutside: () => close() }}
|
||||
use:clickOutside={{
|
||||
onClickOutside: () => closeOnOutsideClick && close()
|
||||
}}
|
||||
>
|
||||
<List gap="md">
|
||||
<div class="flex w-full">
|
||||
|
||||
Reference in New Issue
Block a user