Slate: Can't focus on editor in Firefox when changing state in onFocus handler

[robertson-ja]

Do you want to request a _feature_ or report a _bug_?

Bug

What's the current behavior?

You cannot edit the content of the editor when onFocus has a state change.

https://jsfiddle.net/fj9dvhom/1433/

Mac OS Sierra
Firefox 61.01

I did a bit of debugging and found the problem to be related to rangeCount.
https://github.com/ianstormtaylor/slate/blob/0ceefea2e7c5363df2ee681d3018cf2679f77b64/packages/slate-react/src/components/content.js#L150

Basically, if we setState in onFocus then a render is triggered and that triggers slate to update the selection. At that point, Firefox thinks there is a range count of 1 and triggers a blur event. Chrome thinks the range count is 0 and continues as normal processing the select event and eventually gives focus to the editor.

A basic fiddle demonstrating the browser inconsistency.

https://jsfiddle.net/yftf24g6/43/

What's the expected behaviour?

Firefox works the way Chrome does in the above scenario

[zhujinxuan]

Strange. It can be edited after right click in firefox.

I think the potential reason is that click does not fire a onSelect event automatically in firefox. Then the element is focused without a selection.

[Claire]

This is a major issue for our users on Firefox. Unfortunately we can't tell all of them to right-click to edit a value. Is there any programmatic workaround we can implement until this fix is in?

[benmvp]

We ran into this same problem here at Eventbrite, and thanks to @robertson-ja sleuthing I was able to come up with a workaround!

Solution

The solution was to wrap slate-react editor in a PureComponent component:

import React, {PureComponent} from 'react';
import {Editor} from 'slate-react';

export default class SlateEditor extends PureComponent {
    render() {
        return (<Editor {...this.props} />);
    }
}

Explanation

In our case, the value in state that we're updating via setState is not being passed to slate's <Editor />. The <Editor /> is not changing across re-renders, so really we want to re-render other stuff in our component, but not the <Editor />. The best way to accomplish _that_ is with shouldComponentUpdate and telling it to return false when none of the props passed to <Editor /> haven't changed.

The best way to implement shouldComponentUpdate is with React.PureComponent, so ideally Editor itself would just extend React.PureComponent instead of React.Component. But that requires changing slate-react and is probably not a good idea anyway (explained below). So instead, wrapping it in a PureComponent wrapper as above solves the problem w/o having to change slate-react.

Caveats

In order for the PureComponent solution to work, the props cannot change when the component is re-rendered, because PureComponent uses shallow comparison on its props to determine if it should update. Specifically, the onChange, onBlur, onFocus, etc. handlers cannot change for each render.

Good

class RichTextEditor extends Component {
    onChange = () => {

    }

    onBlur = () => {

    }

    onFocus = () => {

    }

    render() {
        return (
            <SlateEditor 
                value={this.state.value}
                onChange={this.onChange}
                onBlur={this.onBlur}
                onFocus{this.onFocus}
            />
        )
    }
}

Bad

(will break shallow comparison of PureComponent)

class RichTextEditor extends Component {
    onChange() {

    }

    onFocus() {

    }

    render() {
        return (
            <SlateEditor 
                value={this.state.value}
                onChange={this.onChange.bind(this)}
                onBlur={() => {

                }}
                onFocus{this.onFocus.bind(this)}
            />
        )
    }
}

Editor as PureComponent?

Making Editor a PureComponent would mean that it would be doing the shallow comparison of its props & state every time it's re-rendered. If most use cases aren't re-rendering onFocus (like the example), then the shallow comparison work would be a lot of wasted work. And given that we do want to re-render every time the user types, selects, etc. that may have a perceivable performance difference. Also making a change like that would have to be heavily tested. If I were @ianstormtaylor I wouldn't accept such a PR 😃

Hope that helps someone else!!

[benmvp]

BTW - @zhujinxuan once the onBlur stops happening, the onSelect happens as expected 👍🏾

[ianstormtaylor]

Hey folks, don't use onFocus for this. That is the DOM-level event handler. You already have value.selection.isFocused to determine the focused state and render toolbars, etc. based off of that value.

[somanshu]

For Firefox, i was unable to select or focus on the editor. But making a small tweek fixed it.
slate: 0.44.10
slate-react: 0.21.16

// no need to write anything inside it
onFocus = () => {
}

render() {
 return (
  <Editor
    value={this.state.value}
    onChange={this.onChange}
    onFocus={this.onFocus} // add this
 );
}

[murattishkul]

For Firefox, i was unable to select or focus on the editor. But making a small tweek fixed it.
slate: 0.44.10
slate-react: 0.21.16

// no need to write anything inside it
onFocus = () => {
}

render() {
 return (
  <Editor
    value={this.state.value}
    onChange={this.onChange}
    onFocus={this.onFocus} // add this
 );
}

I have the same issue, but cannot solve it using your approach... any other solutions?