Skip to content

Commit 168a35c

Browse files
committed
fix(editor): only fire change event on non-programmatic changes
1 parent b2af5c8 commit 168a35c

File tree

5 files changed

+146
-13
lines changed

5 files changed

+146
-13
lines changed

README.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,12 @@ project element.
811811
| `editor` | `string \| PlaygroundFileEditor` | `undefined` | The editor this bar controls. Either the `<playground-file-editor>` itself, or its `id` in the host scope. |
812812
| `editableFileSystem` | `boolean` | `false` | Allow adding, removing, and renaming files |
813813

814+
### Events
815+
816+
| Event | Description |
817+
| ----------- | ------------------------- |
818+
| `tabchange` | A tab has been activated. |
819+
814820
---
815821

816822
## `<playground-file-editor>`
@@ -861,9 +867,9 @@ A pure text editor based on CodeMirror with syntax highlighting for HTML, CSS, J
861867

862868
### Events
863869

864-
| Event | Description |
865-
| -------- | ------------------------------------ |
866-
| `change` | User made an edit to the active file |
870+
| Event | Description |
871+
| -------- | -------------------------------------------------------------------------------------------------------------------------------------------------- |
872+
| `change` | User made an edit to the active file (note: this event is not fired for programmatic changes to the value property nor for the user changing tabs) |
867873

868874
### Keyboard shortcuts
869875

src/internal/tab-bar.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export class PlaygroundInternalTabBar extends LitElement {
8080
new CustomEvent<{tab?: PlaygroundInternalTab}>('tabchange', {
8181
detail: {tab: undefined},
8282
bubbles: true,
83+
composed: true,
8384
})
8485
);
8586
}

src/internal/tab.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ export class PlaygroundInternalTab extends LitElement {
106106
new CustomEvent<{tab?: PlaygroundInternalTab}>('tabchange', {
107107
detail: {tab: this},
108108
bubbles: true,
109+
composed: true,
109110
})
110111
);
111112
}

src/playground-code-editor.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,8 @@ export class PlaygroundCodeEditor extends LitElement {
339339
@queryAssignedElements({slot: 'extensions', flatten: true})
340340
private _extensionElements!: HTMLElement[];
341341

342-
private _valueChangingFromOutside = false;
342+
// Annotation to mark programmatic changes (not user edits)
343+
private readonly _programmaticChange = Annotation.define<boolean>();
343344
private _diagnosticDecorations: DecorationSet = Decoration.none;
344345
private _diagnosticsMouseoverListenerActive = false;
345346
private _lastTransactions: Transaction[] = [];
@@ -424,6 +425,7 @@ export class PlaygroundCodeEditor extends LitElement {
424425
insert: this.value ?? '',
425426
},
426427
],
428+
annotations: this._programmaticChange.of(true),
427429
});
428430
}
429431
}
@@ -447,14 +449,13 @@ export class PlaygroundCodeEditor extends LitElement {
447449
insert: this.value ?? '',
448450
},
449451
],
452+
annotations: this._programmaticChange.of(true),
450453
});
451454
docState = tempView.state;
452455
this._docCache.set(docKey, docState);
453456
tempView.destroy();
454457
}
455458

456-
this._valueChangingFromOutside = true;
457-
458459
// Replace the entire view with the new editor state. Unlike CM5, CM6
459460
// is modular, and the history stays on the state object rather than
460461
// the editor / view.
@@ -465,8 +466,6 @@ export class PlaygroundCodeEditor extends LitElement {
465466
if (needsHideAndFold) {
466467
void this._applyHideAndFoldRegions();
467468
}
468-
469-
this._valueChangingFromOutside = false;
470469
break;
471470
}
472471
case 'value':
@@ -476,8 +475,6 @@ export class PlaygroundCodeEditor extends LitElement {
476475
}
477476

478477
if (this.value !== view?.state.doc.toString()) {
479-
this._valueChangingFromOutside = true;
480-
481478
// The 'value' property was changed externally and it differs from
482479
// the editor's current document content, so we need to update the
483480
// view model to match.
@@ -492,9 +489,8 @@ export class PlaygroundCodeEditor extends LitElement {
492489
insert: this.value ?? '',
493490
},
494491
],
492+
annotations: this._programmaticChange.of(true),
495493
});
496-
497-
this._valueChangingFromOutside = false;
498494
}
499495
break;
500496
case 'lineNumbers':
@@ -720,9 +716,14 @@ export class PlaygroundCodeEditor extends LitElement {
720716
tr.annotation(Transaction.userEvent) === 'redo'
721717
);
722718

719+
// Check if this change was programmatic (not a user edit)
720+
const isProgrammatic = update.transactions.some(
721+
(tr) => tr.annotation(this._programmaticChange) === true
722+
);
723+
723724
// External changes are usually things like the editor switching which
724725
// file it is displaying.
725-
if (this._valueChangingFromOutside) {
726+
if (isProgrammatic) {
726727
// Apply hide/fold regions when value changes from outside
727728
void this._applyHideAndFoldRegions();
728729
this._showDiagnostics();
@@ -731,6 +732,7 @@ export class PlaygroundCodeEditor extends LitElement {
731732
// Always reapply hide/fold regions after undo/redo
732733
void this._applyHideAndFoldRegions();
733734
}
735+
// Only fire change event for user-initiated edits
734736
this.dispatchEvent(new Event('change'));
735737
}
736738
}

src/test/playground-code-editor_test.ts

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,129 @@ suite('playground-code-editor', () => {
7373
});
7474
});
7575

76+
test('does NOT dispatch change event when switching documentKey and value together', async () => {
77+
const editor = document.createElement('playground-code-editor');
78+
const DOCUMENT_KEY1 = {doc: 1};
79+
const DOCUMENT_KEY2 = {doc: 2};
80+
81+
editor.value = 'document 1';
82+
editor.documentKey = DOCUMENT_KEY1;
83+
container.appendChild(editor);
84+
await editor.updateComplete;
85+
await raf();
86+
87+
let changeCount = 0;
88+
editor.addEventListener('change', () => {
89+
changeCount++;
90+
});
91+
92+
// This simulates what playground-file-editor does when switching files
93+
// Both value AND documentKey change together in a single update
94+
editor.value = 'document 2';
95+
editor.documentKey = DOCUMENT_KEY2;
96+
97+
// Wait for Lit's update cycle
98+
await editor.updateComplete;
99+
await raf();
100+
101+
// Wait for any potential async change events
102+
await wait(100);
103+
104+
// Switch back - this is where the bug manifests in lit.dev
105+
editor.value = 'document 1 modified';
106+
editor.documentKey = DOCUMENT_KEY1;
107+
await editor.updateComplete;
108+
await raf();
109+
await wait(100);
110+
111+
assert.equal(
112+
changeCount,
113+
0,
114+
'Switching documentKey and value together should not fire change event'
115+
);
116+
});
117+
118+
test('does NOT dispatch change event when switching documentKey', async () => {
119+
const editor = document.createElement('playground-code-editor');
120+
const DOCUMENT_KEY1 = {dockey: 1};
121+
const DOCUMENT_KEY2 = {dockey: 2};
122+
123+
editor.value = 'document 1';
124+
editor.documentKey = DOCUMENT_KEY1;
125+
container.appendChild(editor);
126+
await editor.updateComplete;
127+
await raf();
128+
129+
let changeCount = 0;
130+
editor.addEventListener('change', () => changeCount++);
131+
132+
// Switch to a different document - should NOT fire change event
133+
editor.value = 'document 2';
134+
editor.documentKey = DOCUMENT_KEY2;
135+
await editor.updateComplete;
136+
await raf();
137+
138+
assert.equal(
139+
changeCount,
140+
0,
141+
'Switching documentKey should not fire change event'
142+
);
143+
144+
// Switch back to first document - should NOT fire change event
145+
editor.value = 'document 1';
146+
editor.documentKey = DOCUMENT_KEY1;
147+
await editor.updateComplete;
148+
await raf();
149+
150+
assert.equal(
151+
changeCount,
152+
0,
153+
'Switching back to original documentKey should not fire change event'
154+
);
155+
});
156+
157+
test('dispatches change event only for user edits, not programmatic changes', async () => {
158+
const editor = document.createElement('playground-code-editor');
159+
editor.value = 'foo';
160+
container.appendChild(editor);
161+
await editor.updateComplete;
162+
await raf();
163+
164+
const editorInternals = editor as unknown as {
165+
_editorView: EditorView;
166+
};
167+
168+
let changeCount = 0;
169+
editor.addEventListener('change', () => changeCount++);
170+
171+
// Programmatic change - should NOT fire
172+
editor.value = 'bar';
173+
await editor.updateComplete;
174+
await raf();
175+
assert.equal(changeCount, 0, 'No change event after programmatic update');
176+
177+
// User edit via CodeMirror - SHOULD fire
178+
editorInternals._editorView.dispatch({
179+
changes: {
180+
from: 0,
181+
to: editorInternals._editorView.state.doc.length,
182+
insert: 'baz',
183+
},
184+
});
185+
await raf();
186+
assert.equal(changeCount, 1, 'Change event fired after user edit');
187+
188+
// Another programmatic change - should NOT fire
189+
editor.value = 'qux';
190+
await editor.updateComplete;
191+
await raf();
192+
assert.equal(
193+
changeCount,
194+
1,
195+
'No additional change event after another programmatic update'
196+
);
197+
});
198+
76199
suite('history', () => {
77200
let editor: PlaygroundCodeEditor;
78201
let editorInternals: {

0 commit comments

Comments
 (0)