feat(text-editor): add clear method to clear content imperatively

[jecrapo]

Problem

limel-text-editor is a controlled component (it has a value prop), but assigning value only updates the editor when Stencil detects the prop changed. Because the editor debounces its change event, a consumer's bound value can legitimately lag the live document. That produces a case the prop can't express: clearing the editor by assigning '' when the previously rendered prop was already ''.

The clearest example is a chat-style "send box":

1. The user types fast and presses send.
2. The composer flushes, captures the text, emits the message, then sets its bound value back to '' to clear.
3. Stencil batches the "catch-up" value and the clear into one render and commits only the final ''. Since the previously rendered value was already '', the prop goes '' → '' — no change — so the editor never re-renders, @Watch('value') never fires, and the document keeps the sent text on screen.

The editor receives no signal at all in this case, so it cannot be fixed reactively from inside the component. flushPendingChanges() (added in #4128) solved the symmetric read problem for this same use case; this adds the write counterpart.

Change

Add an imperative clear() method to limel-text-editor (and its ProseMirror adapter) that empties the editor's document regardless of the prop's change detection:

• discards any pending debounced change so it can't fire afterward and resurrect old content;
• does nothing if the document is already empty (preserves the caret);
• does not emit a change event (the caller cleared it, so no echo / re-entrancy).

Consumers that previously needed timing workarounds to clear (e.g. deferring by a frame) can call await editor.clear() instead.

// on send:
await editor.flushPendingChanges();        // read: make sure we have the full text
this.converseRequest.emit(this.value);
await editor.clear();                       // write: reliably empty, no timing hack

Backwards compatible (additive method only).

Tests

• Spec: clear resolves, including readonly mode where no adapter is rendered.
• E2E: clear empties content the value prop never caught up to; discards the pending change so no stale change is emitted; does not emit a change event.

Summary by CodeRabbit

• New Features

  • The text editor component now provides a public clear() method to immediately remove all editor content, including when updates are pending/debounced.
  • clear() is designed to avoid emitting the editor’s change event.

• Bug Fixes

  • Improved handling to ensure clearing doesn’t leave the editor in a state where future change notifications could be incorrectly suppressed.

• Tests

  • Added/extended e2e and unit tests covering clear() in default and readonly modes, including discarding pending typed input and preventing stale change payloads.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ea37de49-208e-45c6-ab73-e75af0bb5d91

📥 Commits

Reviewing files that changed from the base of the PR and between df4fe99 and 0f21317.

⛔ Files ignored due to path filters (1)

• etc/lime-elements.api.md is excluded by !etc/lime-elements.api.md

📒 Files selected for processing (4)

• src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx
• src/components/text-editor/text-editor.e2e.tsx
• src/components/text-editor/text-editor.spec.tsx
• src/components/text-editor/text-editor.tsx

---

📝 Walkthrough

Walkthrough

A new imperative clear() method is added to TextEditor and ProsemirrorAdapter. Clearing bypasses value-prop change detection, cancels pending debounced changes, and avoids emitting change. Tests cover clearing, pending-state handling, and readonly behavior.

Changes

Text editor clear API

Layer / File(s)	Summary
clear method and adapter state cleanup src/components/text-editor/text-editor.tsx, src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx	TextEditor.clear() delegates to the adapter. ProsemirrorAdapter.clear() no-ops without a view, cancels pending debounced change emission, skips empty content, and clears via updateView(''). updateView resets suppressChangeEvent in finally.
clear behavior tests src/components/text-editor/text-editor.spec.tsx, src/components/text-editor/text-editor.e2e.tsx	Unit tests assert clear() resolves in default and readonly modes. E2e tests verify clearing when value="" would be skipped, clearing while a debounced change is pending, discarding stale pending text, and emitting no change events after clear.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Lundalogik/lime-elements#4127: Modifies updateView in prosemirror-adapter to resync lastEmittedValue, which directly interacts with the programmatic update suppression path used by clear().
• Lundalogik/lime-elements#4128: Adds flushPendingChanges() and adjusts pending-value logic in the same adapter, closely related to the pending-change cancellation in clear().

Suggested reviewers

• CarlitoSweden
• adrianschmidt

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: adding an imperative clear method to the text editor.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/text-editor-set-value

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-4144/

[civing]

🤖 AI-assisted review — generated with Claude (Claude Code) and shared by @perfryking. Findings are suggestions to verify, not an automated gate.

Nice, focused PR — the clear() framing (over a general setValue) is exactly right, and keeping the debounce untouched while adding an imperative escape hatch is the clean way to solve this. I read the implementation against updateView/watchValue/handleTransaction and verified the two things I'd normally worry about are already handled: updateView('') re-syncs lastEmittedValue to empty (so a later identical edit isn't wrongly deduped) and resets suppressChangeEvent. A few suggestions below, none of them blockers.

Should Fix

1. Add a test that pins the actual differentiator. The three new e2e tests prove clear() empties a diverged document, but none show that the value-prop path can't — which is the whole reason this method exists. Without it, a future change to the prop path could pass all these tests while the real collapse silently regresses. Starting from value: '' is what makes the reassignment a genuine no-op:

test('clear empties content when re-assigning value="" cannot', async () => {
    const { root, waitForChanges, setProps, editor, typeText } =
        await createEditor({ value: '' });

    typeText('hello');
    await vi.waitFor(() => expect(editor.textContent).toBe('hello'));

    // '' -> '' is skipped by change detection, so @Watch never fires
    // and the prop path cannot clear — the gap clear() exists for.
    await setProps({ value: '' });
    await waitForChanges();
    expect(editor.textContent).toBe('hello'); // prop path did NOT clear

    await root.clear();
    await vi.waitFor(() => expect(editor.textContent).toBe(''));
});

2. Consider a try/finally around suppressChangeEvent in updateView. It's pre-existing, but clear() adds a new awaitable, consumer-facing path into updateView, so it's now reachable: if parseAsHTML rejects, suppressChangeEvent stays true and silently kills all future change events for that instance. A finally { this.suppressChangeEvent = false; } closes that off cheaply.

3. Document the no-emit consequence. Agreed that clear() shouldn't emit — that matches how programmatic value assignment behaves on native inputs and on controlled components generally. But the JSDoc states the fact without the implication. One line makes it a contract instead of a surprise: "Does not emit a change event. If you mirror content on change (drafts, validation, dirty state), reset your own copy when calling clear()." Same for the silent no-op in readonly mode (no adapter rendered).

Minor

• The emptiness check serializes the whole doc just for a boolean (currentContent === ''). The converters already expose the canonical emptiness predicate (textContent.trim() === '' && !hasImageNode && !hasCustomElementNode) — cheaper on large docs, same result. The check is correct as-is, just wasteful.
• The spec clear › it resolves asserts almost nothing — in the spec env this.view is falsy so it hits the early return, proving only that an async fn resolves. The readonly variant is the meaningful one; consider dropping the first or swapping it for an "already empty → no-op, no change emitted" case.
• Probably follow-up, not this PR: if a value change interleaves during await clear()'s updateView, two updateView calls can race on suppressChangeEvent/lastEmittedValue. Pre-existing and needs a specific interleaving; the try/finally above narrows it.

Backward-compat

Confirmed purely additive — @beta, no collision with HTMLElement/FormComponent, and the api.md regen is correct. Don't add a release tag to clear (that'd break the flushPendingChanges precedent and trip api-extractor).

[jecrapo]

@civing Thanks, genuinely useful pass. Done in 0b80ee3 (folded into the clear commit) and 0f21317.

Should Fix

• #1 (differentiator test): added. It types into a value: '' editor, asserts setProps({ value: '' }) leaves the text on screen (the '' -> '' no-op the watch skips), then proves clear() empties it. Pins exactly the gap the method exists for.
• #2 (try/finally in updateView): agreed it is a real latent wedge, fixed in its own commit 0f21317. One note on framing: that path was already reachable via watchValue, so this hardens pre-existing code rather than something clear() introduced. Kept it to the one-line finally, no fault-injection test.
• #3 (document the no-emit consequence): added to the JSDoc on both the public method and the adapter, including the readonly silent no-op.

Minor (declining, with reasoning)

• Emptiness predicate vs serialize: leaving as is. clear() runs once per send, so the single serialize is negligible, and swapping to the textContent/image/custom-element predicate risks subtle divergence for no real gain. You flagged it correct-but-wasteful; agreed on both halves.
• Weak clear > it resolves spec: keeping it. It does cover the no-view early return, and the meaningful coverage is in the e2e suite anyway.
• Interleaving race during await clear(): agree it is follow-up and pre-existing; the finally above narrows it. Deferring.

Backward-compat: confirmed, no release tag added to clear.

[civing]

LGTM! 👏

[lime-opensource]

🎉 This PR is included in version 39.36.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[jgroth]

Some follow-up thoughts on clear(). It works and nothing here needs reverting, but I think it's treating a symptom, and I'd like the underlying problem written down before we add a third escape hatch for it (which I think we will).

The core issue is that the editor pretends to be controlled but isn't. It implements FormComponent<string> with a value prop and a change event, so consumers wire them together like a controlled input. But ProseMirror owns the live document; value is only a seed plus an occasional heuristic re-sync in watchValue. The moment the user types, prop and document diverge, and the 300ms debounce on change guarantees the bound value lags the document, so the prop can never be a faithful mirror.

Most of the fiddly state in the adapter is there to paper over that: lastEmittedValue, lastDispatchedValue, suppressChangeEvent, changeWaiting, the echo branch in watchValue. flushPendingChanges() (#4128) and clear() are two more, the read and write halves of one workaround: the binding is lossy, so you need a side door to read the real value and another to force it. The '' -> '' no-op in the description is just where it surfaced to a user.

Two things about clear() that I don't think hold up long term:

• It desyncs the binding. After clear() the editor is empty but the consumer's value is still "hello" and no change fires, so the docstring has to tell people to reset their copy by hand. A controlled component shouldn't do that.
• It only covers the empty case. Setting the editor to any specific value after the prop has diverged hits the same X -> X no-op, so setContent is already implied. That's the third escape hatch.

The alternative is a synchronous input event so the prop can't lag in the first place. handleTransaction already serializes the document on every transaction (to compare against lastEmittedValue), so the cost is already paid per keystroke; the debounce only delays the final change.emit. We can hand consumers the current value synchronously for basically nothing and keep the debounced change for the heavier work. It's the DOM split, input per keystroke for binding and change on settle for save/validation, and we already do it in chip-set (custom change plus custom input, swallowing the native event before re-emitting its own, see handleTextInput).

The two events divide the work. Bind value to input; that keeps the prop level with the document and lets it clear. Use change, which still fires on a 300ms pause or on blur, for the expensive reactions: autosave, validation, dirty state. They don't fight, because watchValue dedups against the current content. Listen to whichever you need, and to both only when you want a live binding and a debounced reaction at once. The one rule is not to drive the binding from change, since that brings back the lag.

Two changes, which have to land together:

• Add an input event on the adapter, emitted synchronously in handleTransaction next to the debounced change, swallowing ProseMirror's native input on this.view.dom like chip-set does. Re-emit through text-editor.
• Rework watchValue to compare the incoming value against the current content first and bail on a match. That makes echo rejection structural instead of heuristic, keeps the caret, and lets us drop lastDispatchedValue and the echo branch. Net less code. Without it the synchronous echo comes back before the debounce settles, gets read as an external set, and cancels the pending change.

The send-box case then works straight through the prop:

<limel-text-editor value={this.draft} onInput={(e) => (this.draft = e.detail)} />
// on send:
this.converseRequest.emit(this.draft);   // already current, no flushPendingChanges()
this.draft = '';                          // "hello" -> "" fires the watch and clears, no clear()

watchValue("") sees the document still holds "hello", cancels the pending change so nothing brings the text back, and clears. Both imperative methods become unnecessary.

None of this is urgent, but the direction matters: limel-text-editor is a declarative component, and a declarative one shouldn't grow an imperative API. Every @Method() is the component admitting its props and events can't express something they should. clear() and flushPendingChanges() are two such admissions and setContent would be a third. The fix is to make the declarative surface honest rather than keep extending the imperative one, and the input event does that, closing the gap that made these methods necessary so we can start retiring them.

@john-traas you mentioned you wanted to do some improvements to the text editor if I understood you correctly. Is some of this part of that, or did you have other changes in mind?

[jecrapo]

@jgroth Nice analysis! Completely agree that it would be better long term to fix the core issue with the text-editor masquerading as a controlled component

[john-traas]

@jgroth yes, it is one major area where we need significant improvement.

My first goal is to improve test coverage to ensure that we don't break any current functionality while doing any re-writes.

[adrianschmidt]

(@)jgroth yes, it is one major area where we need significant improvement.

My first goal is to improve test coverage to ensure that we don't break any current functionality while doing any re-writes.

@john-traas Nice to hear!

You might be interested in some of the recent testing work I've been doing in lime-elements:

• test: add runtime tests for documentation examples #4101
• test: add accessibility checks for documentation examples #4102
• test(menu): browser interaction + visual-regression testing (thin slice) #4147

The first two are mostly just groundwork, but the last of those three is a first shot at writing tests that run the real components, in a real web page served by a real web server, in a real browser, actually clicking around and opening menus and stuff, and not just asserting that classes got applied or removed and attributes updated, but actually taking screenshots and comparing to a baseline from main, to verify the expected behaviour.