ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.96k stars 3.25k forks source link

Slate-React - Unable to focus editor programmatically #2097

Closed Nantris closed 6 years ago

Nantris commented 6 years ago

This is going to be a vague bug report for now as I can't track down exactly how to reproduce this. Programmatic refocus works in all ways in my app except one circumstance.

This bug first appears in Slate-React 0.15.6 and remains in 0.15.9.

Nantris commented 6 years ago

I just spent 2 hours tracking down this bug to 0.15.6, then when I rolled back to 0.15.6 again, it seems to work... Will continue to update.

Nantris commented 6 years ago

It's 0.15.8

I had accidentally used ^0.15.6 for my version, which led to me getting 0.15.9, which has the same problem from 0.15.8

zhujinxuan commented 6 years ago

Can you make a code example? I remember there are some conflicts between react update process and focus. Perhaps you need window.requestAnimation (only works before React v16.3) or use componentDidUpdate to do editor.focus()

Nantris commented 6 years ago

@zhujinxuan I've tried both passing in a change object with a selection with isFocused: true as well as using the myEditor.focus()

I've tried using forceUpdate but no dice there either. The issue is 100% related to 0.15.8 and I tried everything under the sun, including window.requestAnimationFrame and setTimeout.

bryanph commented 6 years ago

I am getting the same issue, It seems to happen quite randomly; something I can focus and something I can't. Haven't found the cause yet.

Nantris commented 6 years ago

@bryanph what version are you using?

I've yet to try versions later than 0.15.9.

I am 100% certain 0.15.8 is where the problem begins for me. I've got no idea how to go about reproducing the issue.

Just a note that 0.15.8 was released August 16th, so we can hopefully find the commit that this problem originates with.

Nantris commented 6 years ago

The only commit between 0.15.7 and 0.15.8 is this one, so the issue must originate here: https://github.com/ianstormtaylor/slate/pull/1975

Nantris commented 6 years ago

I'm trying to understand what exactly these changes do, but I'm afraid I'm a bit too new to the internals of Slate to really grasp the finer points here. @zhujinxuan is there anything in that commit you can identify as a possible cause?

bryanph commented 6 years ago

@Slapbox the issue has something to do with https://github.com/ianstormtaylor/slate/blob/master/packages/slate/src/changes/on-selection.js#L17-L19 this method not being called. When I add a change.focus() in the before onFocus plugin, it works fine.

This isFocused property is used in shouldComponentUpdate in content.js to check whether render() should be called. And somehow this prevents the focus from changing.

bryanph commented 6 years ago

As far as I can see, this was always an issue but just happens to not cause any problems before the commit @Slapbox mentioned. The problem occurs when you call .focus() directly on the DOM element, in this case the selection is not updated in slate. A solution might be as I mentioned, to simply add another change.focus() call in onFocus in before.js or somehow detect .focus() calls when they are happen. The first seems the most straightforward.

Nantris commented 6 years ago

Huh. I'll have to investigate my own code and these slate and slate-react bits more. For me, I spent many hours tracing it to slate-react@0.15.8.

I'm using slate@0.37.5 with slate-react@0.15.7 any everything works as intended for me on these versions.

bryanph commented 6 years ago

@Slapbox #2130 should fix your issue.

ericedem commented 6 years ago

@bryanph @Slapbox Can you guys add a fiddle for reproducing this issue? Thanks!

Nantris commented 6 years ago

@ericedem I probably really can't. The only code I can reproduce it with is proprietary and I can't find a way to reproduce it in similar configurations I could share. I've not tried newer versions of Slate-React. I'm just hoping it ends up fixed after I identified the source PR for the issue, although @bryanph and I have identified different sources, which complicates things.

@zhujinxuan any thoughts on that PR I identified? #1975

Nantris commented 6 years ago

This seems to be fixed with the versions below. @bryanph are you still seeing this issue?

Nantris commented 6 years ago

Okay I was wrong. I am still seeing this.

I believe the root cause is that Slate does not recognize when it's lost focus when I move focus to another element with my keyboard. If I leave Slate with my mouse and then return with my keyboard, it works fine because Slate knows it's been blurred.

Nantris commented 6 years ago

This issue occurs when you focus another element but do not explicitly pass a change to Slate with a Selection as below:

change.select({
  anchor,
  focus,
  isFocused: false // You need this
})

In previous versions of Slate you did not need to explicitly do this.

seunggs commented 5 years ago

Can you elaborate on the above solution? I have a problem with editor.focus() not working.

Nantris commented 5 years ago

Hey @seunggs

This is what seems to work best for me:

    const firstPosition = Point.create({
      key: editor.value.blocks.first().nodes.first().key,
      offset: 0,
    });

    editor.select({
      anchor: firstPosition,
      focus: firstPosition,
      isFocused: true,
    });

    const editorEl = document.querySelector('.editorDiv');
    editorEl.focus();

It's clunky, it might not be necessary to do all those things to get it to work in your situation, but it works for me in Slate 43.

seunggs commented 5 years ago

Hi @Slapbox, thanks for the prompt reply. Unfortunately, that solution didn't work for me. :(

After much agonizing though, I did find something that's completely unexpected that worked. I added onFocus handler to the editor and console logged a message in the handler in an attempt to debug this problem and suddenly editor.focus() started working (!!!).

Unbelievably, that did it. I was trying to implement mention and emoji (with emoji-mart), both of which take the focus away from the editor with external components. After the external actions are done, I tried to run editor.focus() to return the cursor back to the previous position but didn't work.

Unless I'm missing something obvious, this seems like a bug. If anyone can enlighten me as to why this may have fixed the focusing issue, I'd very much appreciate it!

UPDATE: Turns out just adding onFocus handler causes issues on focusing the editor when it's empty. Did more searching around this issue and came across a workaround that seems to be working well so far:

handleFocus = (e, change) => { change.focus() }

ericedem commented 5 years ago

@seunggs you might need to do something like:

<SlateEditor
  onFocus={(event, editor, next) => {

    setTimeout(() => {
      // Change editor state here.
    });    

    return next();
  }}
/>

The problem is that when you are in an event, whether onFocus, onChange, or something else, you are in a synchronous execution stack. So if you make changes to the editor, you could end up with some strange race conditions if you try and make changes to the state of the Editor before all the current changes have a chance to flush on the event loop, and put the editor changes on a later tick of the eventloop (with the setTimeout).

ericedem commented 5 years ago

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

seunggs commented 5 years ago

Hi @ericedem, thanks for the explanation - for me, onFocus={(event, editor, next) => editor.focus()} works as expected. Thanks!

laszlo-horvath commented 2 years ago

Using slate v0.81.0 this is what worked for me:

useMemo(() => {
   Transforms.select(editor, { offset: 0, path: [0, 0] });
}, []);
dylans commented 2 years ago

This is what we use in Plate and it works reliably for us:

https://github.com/udecode/plate/blob/main/packages/core/src/slate/react-editor/focusEditor.ts