New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use native selectionchange event as opposed to React's onSelect #1135
Comments
@YurkaninRyan what prop is the async call updating? |
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 |
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 Ideally though I'd really rather not have to go around React I think. |
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 |
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 facebook/react#5785 |
@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. |
I'm going to try to get it in there! |
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 |
Having a |
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 |
@YurkaninRyan definitely open to that idea! Similar in concept/goal to the |
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 |
Hi @YurkaninRyan.
Isn't slate-react/utils/getPoint sufficient? |
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: #802 |
@YurkaninRyan FWIW, with #1231 a new |
Now that we're also using the native |
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:The text was updated successfully, but these errors were encountered: