fix(extended-text-entry): commit the response before teardown (PIE-1058) - #3130
Merged
CarlaCostea merged 1 commit intoSep 18, 2026
Merged
Conversation
Online Testing loses a written response when the student clicks Next straight after typing: `EditableHtml` commits on blur, `main.jsx` passed that commit into `debounce(this.props.onValueChange, 1500)`, and OT destroys the question view inside that window. No `session-changed` is dispatched, so the host cannot detect the loss - there is no event that failed to arrive. DNAFORM-1097 and DNAFORM-2207 are closed customer defects on this symptom. The session write becomes synchronous and only the dispatch is deferred. The debounce moves from `main.jsx` into the custom element, one debouncer per session field so each keeps its own `complete` semantics. `this._session` now holds the response as soon as the editor commits it, which is what lets any other layer read it - a player's synthesized commit reads exactly that. The delay is unchanged at 1500ms. It coalesces nothing on a blur-only callback, so its only remaining effect is to delay the host's first sight of a committed response; shortening or removing it is a separate decision. `commitPendingSession()` dispatches every deferred event now, and is a no-op when nothing is pending. A player calls it before discarding the element, while the element is still attached and the event can therefore still reach a `document`-level listener. `disconnectedCallback` also flushes, but it runs after removal, so an event dispatched there reaches a listener bound inside the removed subtree and nothing above it. Both paths ship: the method is the one that covers a host listening on `document`, and the teardown flush is what a host on a player with no commit seam gets. The method name is the contract `commitPendingSessions` in `@pie-players/pie-players-shared` looks for, which both legacy players now sweep with (pie-player-components, pie-api-components). Nothing is required of a host. Moving the debounce shortens the round trip through the host and back into `EditableHtml`, where a `props.markup` change calls `editor.commands.setContent` and discards the caret when the normalized markup differs from what the editor holds. For this element the round trip is bounded by the callback firing only on blur, so there is no caret to lose. The other four legacy elements the design doc names - `math-inline`, `math-templated`, `explicit-constructed-response`, `multiple-choice` - have the same shape and are not touched here. Which of them needs the inline flush depends on what still ships from this repository when the go-forward work lands; the design doc's open questions track it. Five tests on the element's event contract, each mutation-tested: dropping the two `flush()` calls fails two of them. They drive the element with no model, so `render()` no-ops and the assertions stay off a React mount of the editor. Design doc: pie-players - docs/prds/session-commit-on-teardown.md Discussion: the Online Testing thread in #pie-team, 2026-09-10 to 2026-09-17
lakatosandrei
requested changes
Sep 18, 2026
| import React from 'react'; | ||
| import { createRoot } from 'react-dom/client'; | ||
| import debug from 'debug'; | ||
| import { debounce } from 'lodash-es'; |
Contributor
There was a problem hiding this comment.
Please use:
import debounce from 'lodash-es/debounce' instead
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Online Testing loses a written response when the student clicks Next straight after typing.
EditableHtmlcommits on blur,main.jsxpassed that commit intodebounce(this.props.onValueChange, 1500), and OT destroys the question view inside that window. Nosession-changedis dispatched, so the host cannot detect the loss. DNAFORM-1097 and DNAFORM-2207 are closed customer defects on this symptom.This repo's half of PIE-1058, converged on with @andrei.lakatos-ext in the #pie-team thread. Player side: pie-players#402, pie-player-components#248, pie-api-components#194. Go-forward elements: pie-elements-ng#147. Design doc:
pie-players—docs/prds/session-commit-on-teardown.md.The session write becomes synchronous and only the dispatch is deferred, one debouncer per session field so each keeps its own
completesemantics.commitPendingSession()— the namecommitPendingSessionsin@pie-players/pie-players-sharedlooks for — flushes on demand;disconnectedCallbackflushes as a backstop.Where to look:
document-level listener. ThedisconnectedCallbackflush runs after removal, reaching only a listener inside the removed subtree, and is what a host on a player with no commit seam gets.element.sessionat sweep time was{ id: "1" }: the response sat inside the React debounce and was never written, so the player's commit saved an empty session.props.markupwould carry.math-inline,math-templated,explicit-constructed-responseandmultiple-choicehave the same shape and are untouched here. OT'skeyupworkaround on.tiptap.ProseMirrorcan be dropped once this ships.