Skip to content

Conversation

@garyg1
Copy link

@garyg1 garyg1 commented Sep 4, 2025

Fixes #1254. In the demo on https://jsonnet.org, if you hit redo/undo for multiple characters, the demo breaks. This seems to be because the browser generates multiple text change events (Chrome 139, MacOS 14.6).

Previous Behavior

https://jsonnet.org/

Screen.Recording.2025-09-02.at.8.12.12.PM.mov

New Behavior

Screen.Recording.2025-09-02.at.8.12.43.PM.mov

@google-cla
Copy link

google-cla bot commented Sep 4, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@garyg1 garyg1 changed the title Add synchronization to web demo (#1254) Fix handling of undo/redo events in web demo (#1254) Sep 4, 2025
@garyg1 garyg1 changed the title Fix handling of undo/redo events in web demo (#1254) Improve handling of undo/redo events in web demo (#1254) Sep 4, 2025
@johnbartholomew johnbartholomew marked this pull request as draft January 21, 2026 14:52
@johnbartholomew johnbartholomew marked this pull request as ready for review January 21, 2026 14:52

for (let textarea_id in editor_by_textarea_id) {
let editor = editor_by_textarea_id[textarea_id];
let startedJobId = 0;
Copy link
Collaborator

@johnbartholomew johnbartholomew Jan 21, 2026

Choose a reason for hiding this comment

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

So if I understand correctly you're using this to serialise processing each of the change events?

I think that's ok. Though I think there's another way to do it without the setTimeout: You can keep a reference to an 'ongoing change' to await, and then chain them. Something like this:

    let ongoingChange = Promise.resolve(true);
    editor.on('change', function() {
        return ongoingChange.then(async () => {
            try {
                // previous change is complete and future changes are queued on our promise
                // do the event processing
            } catch (e) {
                // log the error; if it unwinds further then it'll break the Promise chain
            }
        });
    });

What do you think? (I haven't tried running this to verify it!)

@johnbartholomew
Copy link
Collaborator

Thanks for the contribution! It's great to have a fix for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Demo does not handle undo/redo events

2 participants