fix(rich-text-input): dropdown focus bug #1145
Conversation
| // the reason we keep this here, is because any onClick from our dropdown or our mark buttons | ||
| // will propogate to here. | ||
| if (!props.editor.value.selection.isFocused) { | ||
| setTimeout(focus, 0); |
There was a problem hiding this comment.
setTimeout allows our dropdown button onClick to "win" the race condition
There was a problem hiding this comment.
Just curious, would this also work with setImmediate(focus) instead of setTimeout(focus, 0)? If so, it should be slightly more performant.
There was a problem hiding this comment.
So we want the focus to happen on the next tick essentially?
There was a problem hiding this comment.
@tdeekens yeah. There are lots of questions like this in the Slate slack channel. Usually people recommend to use setTimeout.
There was a problem hiding this comment.
I also wouldn't worry about performance. This only happens when
A) The editor is out of focus
B) The user clicks on a mark button or a dropdown button.
There was a problem hiding this comment.
Sorry for the obvious note: can you add comment please :)
There was a problem hiding this comment.
Also like const = scheduleFocusAfter... would also be cool.
There was a problem hiding this comment.
Err, I added like 5 lines of comments? I can add more...
There was a problem hiding this comment.
Oh wow, I had a completely wrong idea about setImmediate on the browser 🤦♂ This ain't node 😅
|
Percy passing 😌 |
Summary
The bug has to do with calling editor.focus() on any event propagated up to the Toolbar. The inline comment should explain the bug and the fix.
Before
After