Skip to content

Resolving the cursor jumping in the Editor bug #2976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Resolving the cursor jumping in the Editor bug #2976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2715c62
[bouncing-debouncing-bound-homewardbound] Testing moving the bounce t…
SonicScrewdriver Oct 23, 2025
29e248d
[bouncing-debouncing-bound-homewardbound] docs(changeset): Moving deb…
SonicScrewdriver Oct 23, 2025
071a689
[bouncing-debouncing-bound-homewardbound] ain't no state to wait
SonicScrewdriver Oct 23, 2025
fe186b4
[bouncing-debouncing-bound-homewardbound] fixing changeset text
SonicScrewdriver Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mean-moles-poke.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus-editor": minor
---

Resolving a long-standing Perseus Editor cursor jumping bug by removing the debounce on changes in the Editor and moving it upstream.
23 changes: 5 additions & 18 deletions packages/perseus-editor/src/editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ type State = {
// eslint-disable-next-line react/no-unsafe
class Editor extends React.Component<Props, State> {
lastUserValue: string | null | undefined;
deferredChange: any | null | undefined;
widgetIds: any | null | undefined;

underlay = React.createRef<HTMLDivElement>();
Expand Down Expand Up @@ -244,12 +243,6 @@ class Editor extends React.Component<Props, State> {
}
}

componentWillUnmount() {
// TODO(jeff, CP-3128): Use Wonder Blocks Timing API.
// eslint-disable-next-line no-restricted-syntax
clearTimeout(this.deferredChange);
}

getWidgetEditor(
id: string,
type: PerseusWidget["type"],
Expand Down Expand Up @@ -434,17 +427,11 @@ class Editor extends React.Component<Props, State> {
handleChange: (e: React.SyntheticEvent<HTMLTextAreaElement>) => void = (
e: React.SyntheticEvent<HTMLTextAreaElement>,
) => {
// TODO(jeff, CP-3128): Use Wonder Blocks Timing API.
// eslint-disable-next-line no-restricted-syntax
clearTimeout(this.deferredChange);
this.setState({textAreaValue: e.currentTarget.value});
// TODO(jeff, CP-3128): Use Wonder Blocks Timing API.
// eslint-disable-next-line no-restricted-syntax
this.deferredChange = setTimeout(() => {
if (this.state.textAreaValue !== this.props.content) {
this.props.onChange({content: this.state.textAreaValue});
}
}, this.props.apiOptions.editorChangeDelay);
const newValue = e.currentTarget.value;
this.setState({textAreaValue: newValue});
Copy link
Contributor

@handeyeco handeyeco Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flow of content, textAreaValue, and onChange all seem pretty convoluted (not on you, just historically).

The old behavior:

  1. Update textAreaValue
  2. Wait for a timeout (in Editor)
  3. Call onChange which presumably updates props.content
  4. When props.content changes, set textAreaValue to props.content

The new behavior (as I understand it):

  1. Update textAreaValue
  2. Call onChange which presumably updates props.content
  3. Wait for a timeout (in the parent now)
  4. When props.content changes, set textAreaValue to props.content

Why can't it just be:

  1. Call onChange which presumably updates props.content
  2. Use props.content directly

I don't see a good justification for keeping textAreaValue, especially now that we're not debouncing in Editor. If we remove textAreaValue in favor of using props.content directly and still see the issue, I bet there's a bug somewhere where we're unmounting/remounting needlessly because I think React (or maybe the browser) is supposed to keep track of cursor position for us.

if (newValue !== this.props.content) {
this.props.onChange({content: newValue});
}
};

_handleKeyDown: (e: React.KeyboardEvent<HTMLTextAreaElement>) => void = (
Expand Down
Loading