From b8c785d0f87502b23b7f8437ea18efaeaae4ddf7 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Fri, 21 Aug 2026 18:04:53 +0200 Subject: [PATCH] fix: two regressions this branch introduced into shared drawers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found auditing the files here that are used elsewhere in the app. `AppConnectDrawer`: the guard added to stop the inner component being opened twice compared the last resource type against the current one, and reset it to `undefined` on close. The resources page opens the drawer with no resource type, so both sides were `undefined`, the guard matched, and the second opening never handed off — the type list came up empty. The drawer destroys its content on close, so this hit every reopen. Now a flag armed per `open()` call, which cannot collide with a resource type. `ResourceEditorDrawer`: adding `onSaved` had turned the Save handler into `await save(); closeDrawer()`, so the drawer stopped closing immediately and waited for the write. `save()` catches its own errors and never rejects, so that was pure added latency for all ten callers. It now starts the save, closes as it always did, and awaits only to fire `onSaved`. Co-Authored-By: Claude Opus 5 (1M context) --- .../lib/components/AppConnectDrawer.svelte | 25 ++++++++++++------- .../components/ResourceEditorDrawer.svelte | 6 ++++- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/frontend/src/lib/components/AppConnectDrawer.svelte b/frontend/src/lib/components/AppConnectDrawer.svelte index a6a55f7392..8de484929e 100644 --- a/frontend/src/lib/components/AppConnectDrawer.svelte +++ b/frontend/src/lib/components/AppConnectDrawer.svelte @@ -39,21 +39,28 @@ /** `fill` connects into a resource that already exists, instead of creating one. */ export async function open(rt?: string, fill?: string) { fillPath = fill + handedOff = false rtToLoad = rt drawer?.openDrawer?.() } /** - * Once per opening. The reactive statement below re-runs both when `rtToLoad` changes and - * when `appConnectInner` binds, and a second `open()` runs `next()` a second time — which - * walks a resource type opened with one straight past the Connect button and into - * `window.open`. A popup opened from a reactive effect rather than the click is blocked, - * so the drawer then sits on "Finish connection in popup window" with no popup. + * Hand off to the inner component exactly once per opening. The reactive statement below + * re-runs both when `rtToLoad` changes and when `appConnectInner` binds — and it binds + * afresh on every opening, since the drawer destroys its content on close. A second + * `open()` runs `next()` a second time, which walks a drawer opened on a resource type + * straight past the Connect button and into `window.open`; a popup opened from a reactive + * effect rather than from the click is blocked, leaving "Finish connection in popup + * window" with no popup behind it. + * + * A flag rather than the last resource type: `open()` with no argument leaves `rtToLoad` + * undefined, which compares equal to the initial state and would skip the hand-off + * entirely — the resources page opens it that way. */ - let openedFor: string | undefined = undefined + let handedOff = false function onRtToLoadChange(rtToLoad: string | undefined) { - if (openedFor === rtToLoad) return - openedFor = rtToLoad + if (handedOff) return + handedOff = true appConnectInner?.open(rtToLoad) } @@ -68,7 +75,7 @@ bind:this={drawer} on:close={() => { step = 1 - openedFor = undefined + handedOff = false dispatch('close') }} size="700px" diff --git a/frontend/src/lib/components/ResourceEditorDrawer.svelte b/frontend/src/lib/components/ResourceEditorDrawer.svelte index d247654968..296ab693a8 100644 --- a/frontend/src/lib/components/ResourceEditorDrawer.svelte +++ b/frontend/src/lib/components/ResourceEditorDrawer.svelte @@ -158,8 +158,12 @@ unifiedSize="md" startIcon={{ icon: Save }} on:click={async () => { - await resourceEditor?.save() + // Closed before the write is awaited, the way it always was: `save()` toasts its + // own failures and never rejects, so waiting would only add visible lag to every + // caller of this drawer. `onSaved` still fires after the write lands. + const saved = resourceEditor?.save() drawer?.closeDrawer() + await saved onSaved?.() }} disabled={!canSave}