Skip to content
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

Setting the cursor without flicker #125

Open
pbiggar opened this issue May 31, 2019 · 5 comments
Open

Setting the cursor without flicker #125

pbiggar opened this issue May 31, 2019 · 5 comments

Comments

@pbiggar
Copy link
Contributor

pbiggar commented May 31, 2019

Our BS-tea-based editor uses contenteditable so that we can have a cursor in our app. Unfortunately, we are having difficult interactions between the bs-tea model and placing the cursor which causes significant flicker, and I'm wondering if you've any advice on dealing with this.

If we type quickly, we see this:

May-23-2019_13-46-06

What's happening here is that we type "l", "e", "t". We receive events and update our model on each character, which changes the view code. TEA diffs and updates the text node, which as a side-effect causes the browser to reset the cursor to the start of the text node. That's why the cursor is at the start in the gif.

We've been experimenting with different ways of making sure the cursor stays in the same place, but can't get anything right. The code shown in the gif sets the cursor to the right place in a callback (https://gist.github.com/pbiggar/ce30d8028bab49379fb51c2fe11abbca), but the timing is sometimes off and the result in the gif happens.

As an experiment, we tried to do it right after the render loop in Tea_app.ml, and also deep in Vdom.ml (right after Web.Node.set_nodeValue child newText). We found that while we could make it much better, it was extremely brittle and susceptible to minor changes in performance:

May-30-2019 17-18-56

Even things like inserting a console.log made a huge difference to render speed.

Our eventual plan was to implement the Custom node type, but given our prototypes aren't doing well, we're not sure where to go with this. Any thoughts?

@OvermindDL1
Copy link
Owner

contenteditable

Eeehh, that will have a variety of issues regardless, there is a reason there is a huge push to remove it from the spec... ^.^;

If we type quickly, we see this:

However, that's still the same as with textboxes. Remember that the DOM only updates on the next browser frame refresh, it's not immediate, essentially a built-in debounce using the browsers own frame handling.

You could probably mitigate it by turning off async updating by toggling this boolean in the source as at least a test:
https://github.com/OvermindDL1/bucklescript-tea/blob/master/src-ocaml/tea_app.ml#L188
You probably don't want to toggle that permanently though as re-rendering on every-single-message will be very slow with large message counts.

However, this is really the same issue as input boxes, the DOM and your app are both trying to hold the state of a single object, I.E. you are having Two Master's, which is where the conflict comes in with a lot of frameworks. You can't remove the DOM's without removing its ability to control its input either. Thus there are two options:

  1. Let the DOM be master of that state, read it only via events and never ever set it unless you are purposefully wanting to override the user state (good use of key makes this trivial).

  2. You control the state by taking away the DOM's ability to hold the state, this would have to be done by listening to every possible event the node receives and instead of the browser handling it you cancel it and handle it yourself to build up what the DOM should look like (and at that point you can even remove the contenteditable part of it as long as you still make it selectable like via a tabindex). This one is a lot more work though, but gives you absolute control. Personally I'd not recommend it.

A Custom node could abstract some of part 1 of this but using a key makes it easy enough that it's not worth it.

So the question becomes, since you have 2 state controllers for that element, who should be in control?

@OvermindDL1
Copy link
Owner

For note, if turning on realtimeRendering fixes your use then I could probably add an option to event handlers to force a re-render right then and there without waiting for the next frame update? I'd still highly recommend using only one state master though, with two 'owners' you will have occasional issues, regardless of the javascript framework or so.

@OvermindDL1
Copy link
Owner

OvermindDL1 commented May 31, 2019

For note, my way of handling this state master conflicts (though not with contenteditable, that has its own whole world of issues ^.^; ) is to just set a string value in the model as 'marker' for when to update the contenteditable, and put that as the key on it. To update it I just convert it to an integer, if it fails to convert or is above, say, 2 billion then I set it to 0, else I increment it by one then convert it back to a string. I only do this when I specifically want to 'act' on it, like if a user hits a button to 'bold' the selection or so, otherwise I don't touch it, thus meaning the element remains otherwise untouched by the vdom, thus the DOM is the master state control.

@pbiggar
Copy link
Contributor Author

pbiggar commented May 31, 2019

Totally agree that the problem is a standard DOM state problem, and not particular to contenteditable, except that we're dealing with placing cursors.

We're doing option 1: the browser does nothing except send us events. The problem is that we don't know where or how to add the setCursor calls so that anytime we render we have the cursor in the right place. Are you saying the right time is after the requestAnimationFrame? Or should it work if we do it after the set_nodeVale call? Or something else?

@OvermindDL1
Copy link
Owner

In general a setCursor call should be in the frame update callback to work at the right time (which is run right after the DOM is updated).

Honestly, a hack of just allowing a conditional immediate render instead of an async render might work really well... Or perhaps some fake event like 'on-patch' or so on a node can call a callback function that is called when a node is updated, hmm... Different use-cases for those, both could be quite useful though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants