From df4aed127ef1a452e834aa4c6357622aeca6cffb Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 7 Jun 2026 14:08:18 +0800 Subject: [PATCH] fix(actions): make 'Open' (opensInNewTab) feel instant, not stuck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The env-list 'Open' (sso_as_owner) pre-opened a blank tab, then awaited a slow SSO-handoff mint with no feedback — the tab looked frozen and the button gave nothing, so users re-clicked, spawning more blank tabs + requests; when the popup was blocked it silently hijacked the current tab after the long wait. Fixes (ObjectView + RecordDetailView): - Paint a '正在为你打开环境…' spinner into the pre-opened tab immediately, so it never looks blank/frozen during the mint. - Re-entrancy guard: ignore repeat clicks while the same action+record is in flight (no more duplicate tabs/requests). - Blocked-popup fallback: instead of silently navigating the current tab after the wait, show a 'popup blocked → 打开环境' toast with a one-click (fresh-gesture) open, preserving the control-plane tab. Co-Authored-By: Claude Opus 4.8 --- packages/app-shell/src/views/ObjectView.tsx | 54 ++++++++++++++----- .../app-shell/src/views/RecordDetailView.tsx | 29 +++++++++- 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 708e469f1..96bc8702d 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -182,6 +182,10 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an // network write. const persistTimers = useRef>>({}); const persistPending = useRef>>({}); + // Guards against double-firing a server action (e.g. an "Open" that mints a + // slow SSO handoff) — without it, a user who clicks again because the button + // "looks stuck" spawns a second blank tab + a second request. + const serverActionInFlight = useRef>(new Set()); const persistViewPatch = useCallback( (viewIdLocal: string, baseViewDef: Record, patch: Record) => { if (!dataSource?.updateViewConfig || !objectName || !viewIdLocal) return; @@ -1115,22 +1119,32 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an const recordIdField = (action as any).recordIdField || 'id'; const resolvedRecordId = (params as any).recordId ?? _rowRecord?.[recordIdField]; + // Re-entrancy guard: ignore a repeat click while this exact + // action+record is already running — otherwise a user who clicks again + // because the button "looks stuck" spawns another blank tab + request. + const inflightKey = `${targetName}:${resolvedRecordId ?? ''}`; + if (serverActionInFlight.current.has(inflightKey)) { + return { success: false, error: 'Action already in progress' }; + } + serverActionInFlight.current.add(inflightKey); + // ── Popup-blocker workaround (mirrors RecordDetailView) ─────────── - // When `action.opensInNewTab` is set, the handler returns - // `{ redirectUrl }` for the UI to navigate to. Pre-open `about:blank` - // synchronously *before* the await so the user-gesture context is - // preserved and Chrome/Safari don't block the eventual navigation — - // without this, the post-await window.open() on the row-action path - // is dropped as an unsolicited popup. Falls back to navigating the - // current tab if the pre-open is itself blocked. + // Pre-open `about:blank` synchronously *before* the await so the + // user-gesture context is preserved and Chrome/Safari don't block the + // eventual navigation. No 'noopener' — it would null the handle and + // trip the current-tab fallback (double-navigation bug). let preOpenedTab: Window | null = null; if ((action as any).opensInNewTab) { - // NOTE: no 'noopener' — per spec it forces window.open to return - // null even when the tab opens, losing the handle so we'd fall - // through to the popup branch and navigate the *current* (list) - // tab to the redirectUrl (double-navigation bug). We need the - // reference to drive the pre-opened tab to the SSO redirect. - try { preOpenedTab = window.open('about:blank', '_blank'); } catch { preOpenedTab = null; } + try { + preOpenedTab = window.open('about:blank', '_blank'); + // Paint progress immediately so the new tab never looks blank/ + // frozen during the (slow) SSO-handoff mint — the #1 reason + // users thought "Open" was broken and re-clicked. + if (preOpenedTab) { + preOpenedTab.document.write('正在打开… Opening…
正在为你打开环境…
'); + preOpenedTab.document.close(); + } + } catch { preOpenedTab = null; } } try { const baseUrl = import.meta.env.VITE_SERVER_URL || ''; @@ -1175,7 +1189,17 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an // handle; with it the null return would always trip the // current-tab fallback below. try { popup = window.open(redirectUrl, '_blank'); } catch { popup = null; } - if (!popup) window.location.href = redirectUrl; + if (!popup) { + // Popup blocked even with the gesture — DON'T silently + // hijack the control-plane tab after a long wait (the + // jarring "it suddenly navigated away" report). Offer a + // one-click open (a fresh user gesture) instead. + toast('浏览器拦截了弹窗 / Popup blocked', { + description: '点击在新标签页打开环境', + action: { label: '打开环境', onClick: () => { try { window.open(redirectUrl, '_blank'); } catch { window.location.href = redirectUrl; } } }, + duration: 10000, + }); + } } } else if (preOpenedTab) { // opensInNewTab declared but no redirectUrl came back — don't @@ -1188,6 +1212,8 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an const msg = (error as Error).message; toast.error(msg); return { success: false, error: msg }; + } finally { + serverActionInFlight.current.delete(inflightKey); } }, [authFetch, objectDef.name]); diff --git a/packages/app-shell/src/views/RecordDetailView.tsx b/packages/app-shell/src/views/RecordDetailView.tsx index 726304d4d..fc86e2486 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -449,6 +449,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri // server-registered handler name (engine.registerAction). Sends the // current recordId, objectName, and any collected/static params, and the // server resolves the handler (with wildcard '*' fallback) and runs it. + const serverActionInFlight = useRef>(new Set()); const serverActionHandler = useCallback(async (action: ActionDef) => { const targetName = action.target || action.name; if (!targetName) { @@ -458,6 +459,13 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri ? (action.params as Record) : {}; + // Re-entrancy guard: ignore a repeat click while this action+record runs. + const inflightKey = `${targetName}:${pureRecordId ?? ''}`; + if (serverActionInFlight.current.has(inflightKey)) { + return { success: false, error: 'Action already in progress' }; + } + serverActionInFlight.current.add(inflightKey); + // ── Popup-blocker workaround ────────────────────────────────────── // When `action.opensInNewTab` is set, the handler is known to return // `{ redirectUrl: ... }` for the UI to navigate to. We pre-open @@ -475,7 +483,15 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri // tab to the redirectUrl (the double-navigation bug: env opens in a new // tab AND the list/detail page jumps to the now-consumed SSO URL). We // need the reference to drive the pre-opened tab to the SSO redirect. - try { preOpenedTab = window.open('about:blank', '_blank'); } catch { preOpenedTab = null; } + try { + preOpenedTab = window.open('about:blank', '_blank'); + // Paint progress immediately so the new tab isn't blank/frozen during + // the (slow) SSO-handoff mint. + if (preOpenedTab) { + preOpenedTab.document.write('正在打开… Opening…
正在为你打开环境…
'); + preOpenedTab.document.close(); + } + } catch { preOpenedTab = null; } } try { @@ -520,7 +536,14 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri // No 'noopener' so a successful open returns a truthy handle; with // it, the null return would always trip the current-tab fallback. try { popup = window.open(redirectUrl, '_blank'); } catch { popup = null; } - if (!popup) { window.location.href = redirectUrl; } + if (!popup) { + // Don't silently hijack the current tab — offer a one-click open. + toast('浏览器拦截了弹窗 / Popup blocked', { + description: '点击在新标签页打开环境', + action: { label: '打开环境', onClick: () => { try { window.open(redirectUrl, '_blank'); } catch { window.location.href = redirectUrl; } } }, + duration: 10000, + }); + } } } else if (preOpenedTab) { // Handler didn't return a redirectUrl — close the empty tab we @@ -533,6 +556,8 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri const msg = (error as Error).message; toast.error(msg); return { success: false, error: msg }; + } finally { + serverActionInFlight.current.delete(inflightKey); } }, [authFetch, pureRecordId, objectName]);