[email protected]+ regression: broken void selection behavior

[jamesplease]

Description

[email protected] introduced a regression when clicking into void nodes. This issue is still present in the latest version. The change that introduced the regression is https://github.com/ianstormtaylor/slate/pull/4057 .

When navigating to these void nodes through key presses, insertBreak will be called when the Enter key is pressed. When clicked, insertBreak is not called. In both situations, the editor has the exact same selection value.

The expectation is that all nodes would behave consistently upon being selected.

This functioned in [email protected] but started to appear in [email protected]. The version of slate used does not seem to affect the behavior.

Update: A full description of the cause is in this comment below: https://github.com/ianstormtaylor/slate/issues/4301#issuecomment-851008675

Recording

This recording demonstrates the buggy behavior. Note that this video uses this plugin to insert a new node within insertBreak, which makes the bug more visually clear. However, the bug is independent from that plugin, as the CodeSandboxes below demonstrate.

https://user-images.githubusercontent.com/2322305/120079739-55f06a00-c083-11eb-9069-0c50ef06fef6.mov

Sandbox

These two sandboxes are identical other than the slate-react version.

Behaves as expected (0.61.3) Does not behave as expected (0.62.0)

Steps

1. Navigate to the Behaves as expected sandbox
2. Use the arrow keys to navigate to the void
3. Press the Enter key and observe that [editor.insertBreak] is logged to the console, informing you that that method was just called.
4. Now, click into something other than the void, and then click back to the void. Press enter again. Observe that the console log occurs again.
5. Repeat this for the other sandbox, and observe that step 4 does not log to the console.

Expectation

Both versions of this library would behave identically.

Environment

• Slate Version:

  • slate - 0.63.0
  • slate-history - 0.62.0
  • slate-react - 0.62.0+

• Operating System: macOS

• Browser: Chrome 91.0.4472.77, Safari 14.1.1

• TypeScript Version: N/A

---

~~One line of investigation suggests it could be related to https://github.com/ianstormtaylor/slate/pull/4048 , but I'm still investigating.~~ The actual source of this change of behavior is https://github.com/ianstormtaylor/slate/pull/4057

[jamesplease]

Original initial comment (feel free to skip)

I'm trying to debug this by stepping through commits and building the lib, but I can't seem to build anything other than the versioned commits. If anyone has any tips, lmk!

Here's the sort of error I'm getting when building on an arbitrary commit between two versions:

❯ NODE_ENV=production yarn build:rollup
yarn run v1.10.1
$ rollup --config ./config/rollup/rollup.config.js

packages/slate/src/index.ts → packages/slate/dist/index.js...
(!) /Users/jamess/webdev/slate/packages/slate/src/interfaces/editor.ts(1064,3): semantic error TS2322: Type '{ above<T extends Ancestor>(editor: BaseEditor, options?: { at?: BaseRange | Path | BasePoint | undefined; match?: ((node: Node, path: Path) => boolean) | ((node: Node, path: Path) => node is T) | undefined; mode?: "highest" | ... 1 more ... | undefined; voids?: boolean | undefined; }): NodeEntry<...> | undefined; ....' is not assignable to type 'EditorInterface'.
  Object literal may only specify known properties, and 'hasPath' does not exist in type 'EditorInterface'.

I've tried a number of different commits and they all give a similar error.

---

Diff here for debugging purposes

---

~~This appears to be caused by a change in the behavior of normalizeDOMPoint. Likely caused by https://github.com/ianstormtaylor/slate/pull/4048~~

[jamesplease]

Alright, after an absurd number of hours of debugging, I've determined that this new behavior is a consequence of https://github.com/ianstormtaylor/slate/pull/4057 .

Consider a void node that has no inputs/textareas; only a <div/>.

Before that PR, if you click this div, the hidden input that accompanies every void would be the node in the DOM's Selection object. Because of this, if you pressed Enter, all of the events associated with an input changing would occur.

After that PR, if you clicked this div, the <div/> is the node in the Selection. And because the selection is on a <div/> and not an input, pressing Enter fires no selection change events.

---

I understand the cause of the change of behavior now, but what I don't know is what the next steps should be. Is this the expected behavior or is it a bug? Is it even proper usage of Slate to have a void without any input/textarea at all? What is the best solution here?

If anyone has any thoughts on those questions, I'd love to hear them. Also, the use case here is a void that represents a horizontal line in a document.

---

An example commit showing a fix by reverting the earlier can be found here.

@ulion if you have time I'm curious to hear your thoughts on this issue if you have any!

[ulion]

I don't think void expect any specific element inside it, except the default empty text child. I am using some older version of slate-plugins, the img plugin works in both cases of above (I mean if hit enter, insertBreak triggered, via onDOMBeforeInput handler with inputType 'insertParagraph' in chrome, what's your env?), but not official slate, so maybe there is other reason caused your issue?

[jamesplease]

Thanks for the comment, @ulion ! The issue is not with any image plugins or image elements. There are CodeSandboxes showing the regression in the initial comment of this thread. I recognize it can be confusing since my example was forked from the image example, but that was just out of convenience.

The voids in the examples just contain a div other than the default empty text child.

Also, I've reproduced this issue in Chrome and Safari.

[ulion]

So your problem is the div is selectable by click, and arrow will select the empty text, but click does not. I would suggest consider add following css properties to the div to prevent it from being selected. -moz-user-select: none; -webkit-user-select: none; -ms-user-select: none; user-select: none;

[jamesplease]

Hmmm, that wouldn't give me the behavior that I'm expecting. I'm not trying to prevent the div from being selected.

The desired behavior (demonstrated in the 0.61.3) is that clicking the div will set the document selection on the default empty text child, matching the behavior of using arrow keys. In 0.62.0, clicking the div will set the document selection on the div, rather than the empty text child.

For my specific use case, I have a workaround: I manually move the selection to the default empty text child when you click the div.

This issue is more about if this inconsistency between clicking a void and arrowing into a void is the intended behavior of the lib.

[ulion]

select anywhere in void will result same slate point, but not everywhere will got the insertParagraph event, I think it's your job to make sure it happens, either make the div un- user-select -able, or do workaround like you did.

[skolodyazhnyy]

I think I have a problem related to the issue described here. I can't add a new paragraph after a void element. Even in this example https://www.slatejs.org/examples/images you can not add text after second image without deleting it. Not by clicking, not by using arrow keys.

[priyath]

I have faced this exact same problem in a report writing tool that I am working on. Arrow key navigation to void element and pressing "Enter" works (inserts a new paragraph). However, clicking on the void element and pressing enter does not.

I believe the behavior should be consistent across both the click and the arrow key navigation. Using slateJs 0.63

[jamesplease]

@priyath , although I haven't actually tried doing it yet, I was planning to stop using voids altogether and make my own replacement. They're inconsistently implemented and issues like this one show they're liable to break at any time.

[e1himself]

@jamesplease is it possible to implement a replacement? :thinking:

[jamesplease]

No idea tbqh, I paused work on that project. That’s just the last note I left myself

[aaronncfca]

Currently dealing with this as well! I am having similar (but not quite identical) issues with deleteBackward and deleteForward as well.

@jamesplease I'm interested in your workaround (https://github.com/ianstormtaylor/slate/issues/4301#issuecomment-851119496) but unsure how to implement it. Are you overwriting Editor.apply to do something on set_selection? Or are you modifying the DOM selection point?

Update: I found my deleteBackward and deleteForward issues were due to having my entire void node set as contentEditable=false, where I should have allowed the {children} to be contentEditable. I worked around the insertBreak issue by listening for the Enter key in my Editable's onKeyDown handler and calling insertBreak if a void node is selected.