Skip to content

Commit adddcc4

Browse files
authored
fix(panel): restore pre-lock content on unlockPanel — fixes ALL constructor-only premium panels (koala73#3814)
* fix(panel): restore pre-lock content on unlockPanel — fixes ALL constructor-only premium panels Symptom: a premium-gated panel (DeductionPanel reported today; ChatAnalystPanel was the first instance fixed surgically in PR koala73#3797) shows header-only with a vast empty body after a FREE/anon → PRO auth-state cycle. The textarea, buttons, and form chrome are all gone with no way to recover but a reload. Root cause: Panel.unlockPanel() previously called replaceChildren(this.content) to clear the lock-state CTA but never restored the subclass UI. Subclasses whose UI is built ONCE in the constructor (no data-driven re-render path) end up permanently empty after the first showGatedCta → unlockPanel cycle. WEB_PREMIUM_PANELS has 10 candidate panels and 2 are now confirmed broken (chat-analyst, deduction) — playing whack-a-mole one-override-per-panel doesn't scale. Fix: Panel snapshots this.content's child nodes at the moment showLocked / showGatedCta replaces them, and unlockPanel re-attaches those same node instances. The cache holds the ACTUAL DOM nodes (not a re-render seed) so reattaching preserves: - event listeners attached directly to wrapper / textarea / button nodes - subclass references like `this.inputEl` / `this.submitBtn` (still point at the live node) - any in-flight UI state (a half-typed query, a partial stream bubble) Constructor-only subclasses (DeductionPanel + the other 8 in WEB_PREMIUM_PANELS) are repaired transparently with zero per-subclass changes. ChatAnalystPanel's surgical override from PR koala73#3797 becomes redundant belt-and-braces but harmless (its querySelector check finds the restored wrapper and short-circuits the rebuild). Re-entrancy safety: the snapshot is only taken on the FIRST transition into lock state (`if (this._savedContent !== null) return`). A subsequent showLocked / showGatedCta while already locked does not overwrite the cache with the lock-state CTA — covered by a regression test. Test plan - New tests/panel-unlock-restore.test.mts (6 cases) using a minimal `extends Panel` subclass with no override: * Initial mount renders constructor-built UI * showGatedCta → unlockPanel restores nodes by IDENTITY (===), not just by selector — proves the same DOM instance comes back * showLocked → unlockPanel also restores via the same snapshot path * 3 lock/unlock cycles all restore the same node * Re-entrant showLocked while already locked doesn't corrupt the snapshot * Constructor counter stays at 1 across all cycles — proves no rebuild - Existing tests/chat-analyst-panel-unlock.test.mts still 4/4 green — the surgical override from PR koala73#3797 now finds the restored wrapper and short-circuits cleanly. - npx tsc --noEmit clean. Reverting the restore block in unlockPanel breaks exactly the 4 cases that exercise the restore path; the 2 cases that don't (initial mount, never-locked unlock) stay green. Confirms the test bites the right regression. Bundle pattern for the test follows tests/helpers/chat-analyst-panel-harness.mjs but uses an esbuild VIRTUAL ENTRY containing a minimal subclass source string, so the test exercises the REAL Panel base-class restore path without depending on any specific premium panel's full module graph. * fix(panel): destroy clears _savedContent + showGatedCta early-return ordering (PR koala73#3814 review) Two Greptile P2 nits: 1. _savedContent was not cleared in destroy(). Every other DOM-referencing member is explicitly nulled there (pendingContentHtml, tooltipCloseHandler, onTouchMove, …) — _savedContent now matches that pattern. A panel destroyed while still in the locked state no longer retains the detached pre-lock DOM subtree for the lifetime of the Panel instance. 2. showGatedCta() set _locked, hid header siblings, added panel-is-locked, AND snapshotted content BEFORE the if (!entry) return guard for PanelGateReason.NONE. The reason map intentionally doesn't list NONE (it should never reach here in the updatePanelGating flow), but the guard left the panel visually half-locked with no CTA on that path. Moved the side-effects AFTER the guard so the impossible path is a true no-op. Happy paths (FREE_TIER, ANONYMOUS) are behaviorally unchanged. Test: added a 7th case "showGatedCta with an unknown reason is a clean no-op (no half-locked state)" that asserts panel-is-locked class is NOT applied + the wrapper/input nodes stay untouched + no snapshot was taken (verified by post-unlock node identity). Reverting just the reordering flips ONLY this new test red; the other 6 happy-path cases stay green. Confirms the test bites the right behavior. 11/11 tests pass; tsc clean.
1 parent 137e93f commit adddcc4

3 files changed

Lines changed: 489 additions & 11 deletions

File tree

src/components/Panel.ts

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,14 @@ export class Panel {
238238
private retryAttempt = 0;
239239
private _fetching = false;
240240
private _locked = false;
241+
// Snapshot of this.content's children at the moment showLocked /
242+
// showGatedCta replaces them with a lock CTA. unlockPanel re-attaches
243+
// these nodes so subclasses whose UI is constructed once (typically in
244+
// the ctor — chips, input rows, static chrome) don't end up with a
245+
// permanently empty body after a FREE→PRO auth-state cycle. The cache
246+
// holds the actual DOM nodes; reattaching preserves any listeners and
247+
// any subclass references like `this.inputEl`.
248+
private _savedContent: ChildNode[] | null = null;
241249
private _collapsed = false;
242250
private _collapseBtn: HTMLButtonElement | null = null;
243251

@@ -831,6 +839,7 @@ export class Panel {
831839
public showLocked(features: string[] = []): void {
832840
this._locked = true;
833841
this.clearRetryCountdown();
842+
this._snapshotContentForRestore();
834843

835844
for (let child = this.header.nextElementSibling; child && child !== this.content; child = child.nextElementSibling) {
836845
(child as HTMLElement).style.display = 'none';
@@ -869,15 +878,6 @@ export class Panel {
869878
}
870879

871880
public showGatedCta(reason: PanelGateReason, onAction: () => void): void {
872-
this._locked = true;
873-
this.clearRetryCountdown();
874-
875-
// Hide elements between header and content (same as showLocked)
876-
for (let child = this.header.nextElementSibling; child && child !== this.content; child = child.nextElementSibling) {
877-
(child as HTMLElement).style.display = 'none';
878-
}
879-
this.element.classList.add('panel-is-locked');
880-
881881
const config: Record<string, { icon: string; desc: string; cta: string }> = {
882882
[PanelGateReason.ANONYMOUS]: {
883883
icon: lockSvg,
@@ -894,6 +894,20 @@ export class Panel {
894894
const entry = config[reason];
895895
if (!entry) return; // PanelGateReason.NONE should never reach here
896896

897+
// Bail-out done — now commit to the locked state. Doing this AFTER the
898+
// guard avoids a half-locked DOM (header siblings hidden, panel-is-locked
899+
// class set, _savedContent populated) on the acknowledged-impossible
900+
// NONE-reason path. PR #3814 review (Greptile P2).
901+
this._locked = true;
902+
this.clearRetryCountdown();
903+
this._snapshotContentForRestore();
904+
905+
// Hide elements between header and content (same as showLocked)
906+
for (let child = this.header.nextElementSibling; child && child !== this.content; child = child.nextElementSibling) {
907+
(child as HTMLElement).style.display = 'none';
908+
}
909+
this.element.classList.add('panel-is-locked');
910+
897911
const iconEl = h('div', { className: 'panel-locked-icon' });
898912
iconEl.innerHTML = entry.icon;
899913

@@ -913,8 +927,27 @@ export class Panel {
913927
for (let child = this.header.nextElementSibling; child && child !== this.content; child = child.nextElementSibling) {
914928
(child as HTMLElement).style.display = '';
915929
}
916-
// Clear the locked state content
917-
replaceChildren(this.content);
930+
// Restore the pre-lock content if we have it. The saved nodes are the
931+
// ORIGINAL DOM nodes the subclass built — reattaching preserves event
932+
// listeners and any references the subclass holds (this.inputEl etc.),
933+
// and fixes constructor-only subclasses (DeductionPanel,
934+
// ChatAnalystPanel, …) that would otherwise end up with an empty body.
935+
// Fall back to the legacy empty-content behaviour if nothing was saved.
936+
if (this._savedContent !== null) {
937+
replaceChildren(this.content, ...this._savedContent);
938+
this._savedContent = null;
939+
} else {
940+
replaceChildren(this.content);
941+
}
942+
}
943+
944+
// Capture this.content's current child nodes so unlockPanel can put them
945+
// back. Only snapshots on the FIRST transition into a lock state — a
946+
// re-entrant showLocked / showGatedCta must not overwrite the cache with
947+
// the locked-state CTA. The cache is cleared by unlockPanel on restore.
948+
private _snapshotContentForRestore(): void {
949+
if (this._savedContent !== null) return;
950+
this._savedContent = Array.from(this.content.childNodes);
918951
}
919952

920953
public showRetrying(message?: string, countdownSeconds?: number): void {
@@ -1143,6 +1176,10 @@ export class Panel {
11431176
this.contentDebounceTimer = null;
11441177
}
11451178
this.pendingContentHtml = null;
1179+
// Drop the snapshot of pre-lock children so a panel destroyed while
1180+
// still in the locked state doesn't retain the detached DOM subtree
1181+
// for the lifetime of the Panel instance. PR #3814 review (Greptile P2).
1182+
this._savedContent = null;
11461183

11471184
if (this.tooltipCloseHandler) {
11481185
document.removeEventListener('click', this.tooltipCloseHandler);
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
// Bundles a tiny `extends Panel` subclass with no constructor-only-UI
2+
// override, so tests can prove the BASE-CLASS unlock-restore behavior
3+
// without depending on any specific premium panel's implementation.
4+
//
5+
// Mirrors the structure of chat-analyst-panel-harness.mjs (same stubs,
6+
// same browser-environment shim) but the esbuild entry is a virtual
7+
// in-memory file rather than a real source file.
8+
9+
import { build } from 'esbuild';
10+
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
11+
import { tmpdir } from 'node:os';
12+
import { dirname, join, resolve } from 'node:path';
13+
import { fileURLToPath, pathToFileURL } from 'node:url';
14+
import { createBrowserEnvironment } from './runtime-config-panel-harness.mjs';
15+
16+
const __dirname = dirname(fileURLToPath(import.meta.url));
17+
const root = resolve(__dirname, '..', '..');
18+
19+
function snapshotGlobal(name) {
20+
return {
21+
exists: Object.prototype.hasOwnProperty.call(globalThis, name),
22+
value: globalThis[name],
23+
};
24+
}
25+
26+
function restoreGlobal(name, snapshot) {
27+
if (snapshot.exists) {
28+
Object.defineProperty(globalThis, name, {
29+
configurable: true,
30+
writable: true,
31+
value: snapshot.value,
32+
});
33+
return;
34+
}
35+
delete globalThis[name];
36+
}
37+
38+
function defineGlobal(name, value) {
39+
Object.defineProperty(globalThis, name, {
40+
configurable: true,
41+
writable: true,
42+
value,
43+
});
44+
}
45+
46+
async function loadMinimalPanel() {
47+
const tempDir = mkdtempSync(join(tmpdir(), 'wm-minimal-panel-'));
48+
const outfile = join(tempDir, 'MinimalPanel.bundle.mjs');
49+
50+
// Virtual entry source — defines a minimal `extends Panel` subclass that
51+
// builds its UI ONCE in the constructor (the exact shape that triggers
52+
// the unlock-wipe bug). Imports the REAL Panel base class so the test
53+
// exercises the actual unlock/restore code path.
54+
const panelImportPath = resolve(root, 'src/components/Panel.ts').replace(/\\/g, '/');
55+
const domUtilsPath = resolve(root, 'src/utils/dom-utils.ts').replace(/\\/g, '/');
56+
const virtualEntrySource = `
57+
import { Panel } from '${panelImportPath}';
58+
import { h, replaceChildren } from '${domUtilsPath}';
59+
60+
// Module-level counter so a test can prove the ctor (and thus the
61+
// initial DOM build) runs ONLY ONCE per panel instance — the whole
62+
// point of base-class restore is to avoid rebuilding.
63+
export let constructorRunCount = 0;
64+
export function resetConstructorRunCount() { constructorRunCount = 0; }
65+
66+
export class MinimalConstructorOnlyPanel extends Panel {
67+
static MARKER_CLASS = 'minimal-test-wrapper';
68+
static INPUT_CLASS = 'minimal-test-input';
69+
70+
constructor() {
71+
super({ id: 'minimal-test', title: 'Minimal Test' });
72+
constructorRunCount += 1;
73+
// Build UI ONCE — no buildUI() method, no override of unlockPanel.
74+
// This mirrors DeductionPanel's shape (src/components/DeductionPanel.ts:30-77).
75+
const input = h('textarea', { className: MinimalConstructorOnlyPanel.INPUT_CLASS });
76+
const wrapper = h('div', { className: MinimalConstructorOnlyPanel.MARKER_CLASS }, input);
77+
replaceChildren(this.content, wrapper);
78+
}
79+
}
80+
`;
81+
82+
const stubModules = new Map([
83+
['i18n-stub', `export function t() { return ''; }`],
84+
['runtime-stub', `export function isDesktopRuntime() { return false; }`],
85+
['tauri-bridge-stub', `export function invokeTauri() { return Promise.reject(new Error('not wired in test')); }`],
86+
['analytics-stub', `export function trackPanelResized() {}`],
87+
['ai-flow-settings-stub', `export function getAiFlowSettings() { return { badgeAnimation: false }; }`],
88+
['runtime-config-stub', `export function getSecretState() { return { present: true }; }`],
89+
['panel-gating-stub', `
90+
export const PanelGateReason = Object.freeze({
91+
NONE: 'none',
92+
ANONYMOUS: 'anonymous',
93+
FREE_TIER: 'free_tier',
94+
});
95+
`],
96+
['checkout-stub', `export function startCheckout() {}`],
97+
['products-stub', `export const DEFAULT_UPGRADE_PRODUCT = 'pro';`],
98+
['virtual-entry', virtualEntrySource],
99+
]);
100+
101+
const aliasMap = new Map([
102+
['@/services/i18n', 'i18n-stub'],
103+
['../services/i18n', 'i18n-stub'],
104+
['@/services/runtime', 'runtime-stub'],
105+
['../services/runtime', 'runtime-stub'],
106+
['@/services/tauri-bridge', 'tauri-bridge-stub'],
107+
['../services/tauri-bridge', 'tauri-bridge-stub'],
108+
['@/services/analytics', 'analytics-stub'],
109+
['@/services/ai-flow-settings', 'ai-flow-settings-stub'],
110+
['@/services/runtime-config', 'runtime-config-stub'],
111+
['@/services/panel-gating', 'panel-gating-stub'],
112+
['@/services/checkout', 'checkout-stub'],
113+
['@/config/products', 'products-stub'],
114+
['virtual:minimal-entry', 'virtual-entry'],
115+
]);
116+
117+
const plugin = {
118+
name: 'minimal-panel-test-stubs',
119+
setup(buildApi) {
120+
buildApi.onResolve({ filter: /.*/ }, (args) => {
121+
const target = aliasMap.get(args.path);
122+
return target ? { path: target, namespace: 'stub' } : null;
123+
});
124+
125+
buildApi.onLoad({ filter: /.*/, namespace: 'stub' }, (args) => ({
126+
contents: stubModules.get(args.path),
127+
loader: 'ts',
128+
resolveDir: root,
129+
}));
130+
},
131+
};
132+
133+
const result = await build({
134+
entryPoints: [{ in: 'virtual:minimal-entry', out: 'MinimalPanel.bundle' }],
135+
bundle: true,
136+
format: 'esm',
137+
platform: 'browser',
138+
target: 'es2020',
139+
write: false,
140+
plugins: [plugin],
141+
});
142+
143+
writeFileSync(outfile, result.outputFiles[0].text, 'utf8');
144+
145+
const mod = await import(`${pathToFileURL(outfile).href}?t=${Date.now()}`);
146+
return {
147+
MinimalConstructorOnlyPanel: mod.MinimalConstructorOnlyPanel,
148+
getConstructorRunCount: () => mod.constructorRunCount,
149+
resetConstructorRunCount: mod.resetConstructorRunCount,
150+
cleanupBundle() {
151+
rmSync(tempDir, { recursive: true, force: true });
152+
},
153+
};
154+
}
155+
156+
export async function createMinimalPanelHarness() {
157+
const originalGlobals = {
158+
document: snapshotGlobal('document'),
159+
window: snapshotGlobal('window'),
160+
localStorage: snapshotGlobal('localStorage'),
161+
requestAnimationFrame: snapshotGlobal('requestAnimationFrame'),
162+
cancelAnimationFrame: snapshotGlobal('cancelAnimationFrame'),
163+
navigator: snapshotGlobal('navigator'),
164+
HTMLElement: snapshotGlobal('HTMLElement'),
165+
HTMLButtonElement: snapshotGlobal('HTMLButtonElement'),
166+
Node: snapshotGlobal('Node'),
167+
};
168+
const browserEnvironment = createBrowserEnvironment();
169+
const MiniNode = Object.getPrototypeOf(browserEnvironment.HTMLElement.prototype).constructor;
170+
171+
defineGlobal('document', browserEnvironment.document);
172+
defineGlobal('window', browserEnvironment.window);
173+
defineGlobal('localStorage', browserEnvironment.localStorage);
174+
defineGlobal('requestAnimationFrame', browserEnvironment.requestAnimationFrame);
175+
defineGlobal('cancelAnimationFrame', browserEnvironment.cancelAnimationFrame);
176+
defineGlobal('navigator', browserEnvironment.window.navigator);
177+
defineGlobal('HTMLElement', browserEnvironment.HTMLElement);
178+
defineGlobal('HTMLButtonElement', browserEnvironment.HTMLButtonElement);
179+
defineGlobal('Node', MiniNode);
180+
181+
let MinimalConstructorOnlyPanel;
182+
let getConstructorRunCount;
183+
let resetConstructorRunCount;
184+
let cleanupBundle;
185+
try {
186+
({
187+
MinimalConstructorOnlyPanel,
188+
getConstructorRunCount,
189+
resetConstructorRunCount,
190+
cleanupBundle,
191+
} = await loadMinimalPanel());
192+
} catch (error) {
193+
restoreGlobal('document', originalGlobals.document);
194+
restoreGlobal('window', originalGlobals.window);
195+
restoreGlobal('localStorage', originalGlobals.localStorage);
196+
restoreGlobal('requestAnimationFrame', originalGlobals.requestAnimationFrame);
197+
restoreGlobal('cancelAnimationFrame', originalGlobals.cancelAnimationFrame);
198+
restoreGlobal('navigator', originalGlobals.navigator);
199+
restoreGlobal('HTMLElement', originalGlobals.HTMLElement);
200+
restoreGlobal('HTMLButtonElement', originalGlobals.HTMLButtonElement);
201+
restoreGlobal('Node', originalGlobals.Node);
202+
throw error;
203+
}
204+
205+
return {
206+
document: browserEnvironment.document,
207+
createPanel: () => new MinimalConstructorOnlyPanel(),
208+
getConstructorRunCount,
209+
resetConstructorRunCount,
210+
cleanup() {
211+
cleanupBundle();
212+
restoreGlobal('document', originalGlobals.document);
213+
restoreGlobal('window', originalGlobals.window);
214+
restoreGlobal('localStorage', originalGlobals.localStorage);
215+
restoreGlobal('requestAnimationFrame', originalGlobals.requestAnimationFrame);
216+
restoreGlobal('cancelAnimationFrame', originalGlobals.cancelAnimationFrame);
217+
restoreGlobal('navigator', originalGlobals.navigator);
218+
restoreGlobal('HTMLElement', originalGlobals.HTMLElement);
219+
restoreGlobal('HTMLButtonElement', originalGlobals.HTMLButtonElement);
220+
restoreGlobal('Node', originalGlobals.Node);
221+
},
222+
};
223+
}

0 commit comments

Comments
 (0)