react-prosemirror icon indicating copy to clipboard operation
react-prosemirror copied to clipboard

CJK Input stuttering with `dispatchTransaction`

Open bentongxyz opened this issue 2 years ago • 13 comments

I noticed that using the README.md example code:

import { ProseMirror } from "@nytimes/react-prosemirror";
import { useState } from "react";
import { EditorState } from "prosemirror-state";
import { schema } from "prosemirror-schema-basic";

export function ProseMirrorEditor() {
  const [mount, setMount] = useState<HTMLElement | null>(null);
  const [editorState, setEditorState] = useState(
    EditorState.create({schema})
  );

  return (
    <ProseMirror
      mount={mount}
      state={editorState}
      dispatchTransaction={(tr) => {
        setEditorState((s) => s.apply(tr));
      }}
    >
      <div ref={setMount} />
    </ProseMirror>
  );
}

the CJK input in browser is stuttering.

Screencast from 2023-06-26 02-37-59.webm

However, if I remove dispatchTransaction prop, i.e.

import { ProseMirror } from "@nytimes/react-prosemirror";
import { useState } from "react";
import { EditorState } from "prosemirror-state";
import { schema } from "prosemirror-schema-basic";

export function ProseMirrorEditor() {
  const [mount, setMount] = useState<HTMLElement | null>(null);
  const [editorState, setEditorState] = useState(
    EditorState.create({schema})
  );

  return (
    <ProseMirror
      mount={mount}
      state={editorState}
    >
      <div ref={setMount} />
    </ProseMirror>
  );
}

it behaves normally:

Screencast from 2023-06-26 02-38-52.webm

Any idea what is the source of problem?

bentongxyz avatar Jun 25 '23 18:06 bentongxyz

Ah, this is probably related to the IME composition issues that were brought up in #49? If so, I believe it should be resolved by the new architecture in #47! If you want to give it a shot, I published the latest as 0.3.0-beta.0 on npm.

smoores-dev avatar Sep 19 '23 21:09 smoores-dev

I conducted tests on version "@nytimes/react-prosemirror": "^0.3.0", and the issue still persists.

Ah, this is probably related to the IME composition issues that were brought up in #49? If so, I believe it should be resolved by the new architecture in #47! If you want to give it a shot, I published the latest as 0.3.0-beta.0 on npm.

totorofly avatar Nov 10 '23 20:11 totorofly

You'll have to specifically use "@nytimes/react-prosemirror": "0.3.0-beta.0" to get the new architecture!

smoores-dev avatar Nov 10 '23 20:11 smoores-dev

You'll have to specifically use "@nytimes/react-prosemirror": "0.3.0-beta.0" to get the new architecture!

image

I tried switching to version 0.3.0-beta.0. The input indeed became possible, but when I used an IME to input text and confirmed it with the space bar, the cursor remained in front of the entered text. image image

Additionally, if I tried to move the cursor to the right, I found it couldn't move at all. Any attempt to select the text resulted in it being forcibly deselected, with the cursor always staying at the leftmost position of the entered text. Under these circumstances, if I turned off the IME and tried typing in English letters, an error occurred: 'Unhandled Runtime Error Not Found Error: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.'

image

totorofly avatar Nov 10 '23 20:11 totorofly

Darn, ok, thanks for the report! I'll take a look; it's possible that that version just doesn't have the latest code, because I did actually test with Japanese IME input specifically.

Actually, the latest code is deployed here: https://nytimes.github.io/react-prosemirror/. If you have a moment, would you give that a shot, and if you run into issues, could you take a screen recording of what you're seeing? I did just confirm that I can insert Japanese characters with IME input on my computer/browser at least.

smoores-dev avatar Nov 10 '23 21:11 smoores-dev

I am very willing to cooperate with testing. However, when I used the address you provided and turned off the IME to use the system's default input, entering any letter or number resulted in the following error: image image

totorofly avatar Nov 10 '23 21:11 totorofly

Ah! I was able to reproduce this on Chrome; it seems to be a difference in how Chrome and Firefox handle... something! Not sure yet, but this is very helpful. I'll continue to dig into this; thank you for your assistance in pinpointing this!

smoores-dev avatar Nov 10 '23 21:11 smoores-dev

Ah! I was able to reproduce this on Chrome; it seems to be a difference in how Chrome and Firefox handle... something! Not sure yet, but this is very helpful. I'll continue to dig into this; thank you for your assistance in pinpointing this!

On the address you provided, whether I use IME or turn it off, I can input and output content normally on lines that already have text. I can insert a Chinese characters with IME input.

image

totorofly avatar Nov 10 '23 21:11 totorofly

Oh great, that's good to know! Starting a new line is definitely its own edge case in this situation

smoores-dev avatar Nov 10 '23 21:11 smoores-dev

Oh great, that's good to know! Starting a new line is definitely its own edge case in this situation

Based on the current test results, 1) if a new blank line is added by pressing Enter, the same error as mentioned above will occur, regardless of whether the IME is turned on or off, as long as there is input; 2) if a line break is made from the middle of the content in the second paragraph, creating a line with content, then no problem occurs.

totorofly avatar Nov 10 '23 21:11 totorofly

Is there going to be a new release coming soon? I've been using Tiptap, but I've found that it supports Vue better than React, and I'm using Next.js based on React. So, I've been looking for alternatives. If there's a new release, even a Beta version, could you provide a version for internal testing?

totorofly avatar Nov 14 '23 02:11 totorofly

That's the plan! But I can't make any promises about timing, unfortunately. We're still working out some significant high level decisions about the approach.

smoores-dev avatar Nov 14 '23 03:11 smoores-dev

Hey @saranrapjs, while you're at it: @ilyaGurevich took a stab at this a little while back but it seems like there are still issues with IME input in the main branch. I haven't sat down to try to think through this, but this seems to be another thing that works fine with NYT's custom implementation ("simple" node views) but isn't working here!

smoores-dev avatar Nov 14 '23 15:11 smoores-dev