Slate: Use native selectionchange event as opposed to React's onSelect

[YurkaninRyan]

I want to take a shot at implementing this but before I do I want to see what people think.

Currently, I believe Slate is relying on React's onSelect event. The problem with that is that it only fires when the user releases their mouse. This causes a desync between the selection state in the editor state and what is actually on the screen mid-highlight. Consider this workflow:

1. User types
2. Some kind of polling mechanism sets of some async actions, (in my case this would be an autosaving service)
3. User starts highlighting text
4. mid-highlight a prop gets updated because of the async autosave call
5. Editor rerenders and resets the selection, causing it to collapse

[ianstormtaylor]

@YurkaninRyan what prop is the async call updating?

[YurkaninRyan]

It's updating my isAutosaving prop, which doesn't go directly on the editor, but causes the wrapper around the editor to rerender, the editor seems to rerender along with it everytime

[ianstormtaylor]

Open to the idea of this. Consider another case that might help you think about it:

If we have collaborative editing, there are going to be cases where new operations come in from the server, and applying those shouldn't cause the scrollToSelection logic to fire. However, they should cause the selection to be updated if the new changes would have shifted the selection at all.

Ideally though I'd really rather not have to go around React I think.

[ianstormtaylor]

Put differently, right now we have a 1:1 connection between the editor re-rendering and the selection being updated, but I think not all situations will be okay with that. So maybe there's something to investigate about making that logic smarter or more selective, and maybe it doesn't end up needing to hack around React? Not sure.

Another idea would be to augment React's event-handling if it's not ideal for this case, if they were open to it. They've been open to onSelect-related changes in the past I think.

[YurkaninRyan]

What we don't have though is a 1:1 with the user updating the selection and the selection model updating

I found this issue but it doesn't seem to be gaining much traction https://github.com/facebook/react/issues/5785

[ianstormtaylor]

@YurkaninRyan very fair. I'd love to see a solution that keeps it all inside React, by getting a PR merged that adds the new event to React's event.

[YurkaninRyan]

I'm going to try to get it in there!

[YurkaninRyan]

For anyone that comes here and has a similar problem here's the status, i'm trying to get this in at React core, and i'm waiting for some feedback on my approach from the React team.

But, if you need to fix this problem now, here is what you can do.

Outside your editor, when your editor mounts attach a selectionchange event listener.
When you're editor fires a mousedown event, set a flag that allows for that selection change event to make changes.
When your editor fires mouseup set that flag to false.
When selectionchange fires, and the editor is the active element, create a selection state, and use change's select to set it.

[danburzo]

Having a onselectionchange that picks up on the DOM event would be pretty useful in the codebase, we may be able to catch some IME events that replace some text, if they first trigger a selection change.

[YurkaninRyan]

One thing i'm struggling with is turning the window selection into a valid slate selection, I had to write a lot of custom logic to prevent errors from being thrown. Would it be worth some time to expose that selection logic somehow? I'm thinking of an API that you call that grabs the current window selection and just returns a slate selection or null if the selection isn't valid @ianstormtaylor

[ianstormtaylor]

@YurkaninRyan definitely open to that idea! Similar in concept/goal to the findDOMNode helper.

[YurkaninRyan]

Perfect, that's exactly how I saw it, i'm going to port over what I have and try to think of ways to test it

[AlbertHilb]

Hi @YurkaninRyan.

One thing i'm struggling with is turning the window selection into a valid slate selection

Isn't slate-react/utils/getPoint sufficient?

[echenley]

As a temporary workaround, I considered trying something to the effect of this (as suggested by @YurkaninRyan and @AlbertHilb):

import getPoint from 'slate-react/lib/utils/get-point'
// ...

class SlateEditor extends React.Component {
  // ...
  componentDidMount() {
    document.addEventListener('selectionchange', this.onSelectionChange)
  }

  componentWillUnmount() {
    document.removeEventListener('selectionchange', this.onSelectionChange)
  }

  onSelectionChange(e: Event) {
    const { state } = this.props
    const { editor, isDragging } = this

    // isDragging is toggled on mousedown and mouseup
    if (editor && isDragging) {
      const selection = document.getSelection()

      if (!selection) {
        return null
      }

      const { anchorNode, anchorOffset, focusNode, focusOffset } = selection

      if (!anchorNode || !focusNode) {
        return null
      }

      const anchorPoint = getPoint(anchorNode, anchorOffset, state, editor)
      const focusPoint = getPoint(focusNode, focusOffset, state, editor)

      if (!anchorPoint || !focusPoint) {
        return null
      }

      const newSelection = Selection.create({
        anchorKey: anchorPoint.key,
        anchorOffset: anchorPoint.offset,
        focusKey: focusPoint.key,
        focusOffset: focusPoint.offset,
      })

      return this.onChange(state.change.select(newSelection).focus())
    }
  }
  // ...
}

As a proof of concept it does appear to work, though it's pretty quirky as far as where the cursor ends up after the text is inserted. I am not sure I'm comfortable putting this hack in production for this reason. Also, onSelectionChange is fired like a zillion times per second, so probably worth debouncing it.

Related issue: https://github.com/ianstormtaylor/slate/issues/802

[ianstormtaylor]

@YurkaninRyan FWIW, with #1231 a new findRange helper is exposed that returns a Slate Range (previously Selection) from a DOM Selection/Range.

[ianstormtaylor]

Now that we're also using the native onbeforeinput event, circumventing React's handling of it in newer browsers, I think it would make sense to handle onselectionchange in the same way. If anyone was down to make a PR with this fix I'd gladly accept it!