-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Components bind-value-oninput can lose cursor position with fast typing #8204
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
Comments
This issue was assigned to me so I would write up some thoughts on resolution, not with the expectation of a fix in preview 6. So here are my thoughts. @javiercn has already written up some thoughts on the general issue, but in this post I want summarise it from the point of view of typing, since that's probably the worst case. The problemThis is a general problem affecting any case where the DOM can get out of sync with the underlying rendertree (e.g., because the user modified the state of a form input). But it's worse for server-side Blazor, because given the async-ness, there are extra opportunities to be out of sync due to time spent in flight. Common example which is at the root of this issue here:
So not only does it flicker ("ac" then "ab" then "ac"), it entirely lost the "b" keystroke. The solutionI think there are two major different approches we could take: one-way bindings, or correcting the render tree. One-way bindingFor any form element where you don't want to have custom logic that mutates the state, you only want the state to be mutated directly by UI events. It makes no sense to have the server keep sending back instructions to update the form field state when it should already be in the correct state. If it was possible to do a one-way binding (client-to-server, or you might say JS-to-.NET), the problem would disappear in the typical typing case.
Overall I don't think this is a good solution, mostly because it's nasty that developers would have to start making decisions about this for every form field. I'm only writing it up here because a while ago I speculated it might be the right solution, and I no longer think so. Correcting the render treeThis is basically @javiercn's original idea with a few tweaks. Basically, we have some specialised behavior for the known primitive form elements. When an event comes in, the JavaScript-side code can optionally attach some info to describe what the latest state of the associated DOM element is (e.g., checkbox checkedness, text field text value). Then the .NET-side code uses the event handler ID to find the corresponding element data in the previous rendertree, and uses the incoming info to actually mutate the state of that old tree to represent the actual state in the UI now. Then, when the diff algo does its thing, it will automatically determine whether the UI element is already in the right state (which for normally-bound text fields, checkboxes, etc. it will be) and in that case won't even issue instructions to update it. But if somehow we find it's not in the right state now (e.g., custom setter logic), then the diff will produce an instruction to update the UI.
Overall, this sounds promising enough at least to be worth prototyping. But we still can't escape the laws of mathematicsIf the server has custom logic that is meant to modify the form field state, and the updates occur asynchronously, there can be cases where it's impossible to know how to reconcile the changes. Example:
So again, the I'm extremely unkeen on introducing APIs for resolving merge conflicts. For our initial release at least, I'd rather either:
Also cc @rynowak for his input. |
Unassigning myself and sending on to preview 7. |
I think we have to do this for server-side blazor. With the amount of time we have left in the release, we have to look at the minimal viable solution for each issue unless it's going to prevent us from doing something better in the future. |
Unifying all the related issues into this one |
I've put a working proof-of-concept fix for this here: https://github.com/aspnet/AspNetCore/compare/stevesa/poc-handle-fast-events. It's a combination of Javier's idea about patching the existing render trees, plus a new idea about tracking the chain of replacements for event handler IDs. This fixes fast typing as well as a whole range of similar "async events" diffing issues. It's hugely beneficial for making server-side Blazor behave like you'd expect in high-latency situations. Turning this into a production-ready implementation will be 2-3 days work, assuming good test coverage. |
That's great news. |
Turns out you could beat the laws of physics 😄 |
Unfortunately my solution only addresses the thing that's possible to solve. It doesn't do anything for the "merge conflict" case, which will continue to operate on a last-write-wins basis. This only occurs if you have server-side code that mutates the data the user is editing while they are editing it, so not in default |
Yeah, I think that in the end we will have to give guidance to people building server-side apps |
…by patching old tree. Fixes #8204
…by patching old tree. Fixes #8204
…by patching old tree. Fixes #8204
…by patching old tree. Fixes #8204
…by patching old tree. Fixes #8204
Repro:
<input bind-value-oninput="@SomeString" />
X
into the inputX
)Expected: Keystrokes typed should appear only at the cursor position, so the
X
should remain at the endActual: A few keystrokes appear before the
X
, but then the cursor jumps to the end, and subsequent keystrokes are placed after theX
I think this happens because of how
bind
does a 2-way binding. On each keystroke, it not only (1) reads the updated value and writes it toSomeString
, but also (2) tries to re-apply the value fromSomeString
to the input. When it does (2), as long as the value in the input already matches the string, it's a no-op and gets skipped, so normally we don't do anything that moves the cursor.However, because of the asynchrony with Razor Components, there might be more keystrokes between (1) and (2), so the value we're applying in (2) no longer matches the contents of the input. We don't lose the keystrokes (we already did some work on that), but the fact that
input.value !== SomeString
means we do write both the previous and then current value to the input. The fact that we're no longer no-opping this write means now the cursor implicitly moves to the end.Suggested fix: In
BrowserRenderer.ts
, intryApplyValueProperty
, we should add more logic to preserve the selectionStart/selectionEnd properties of input/textarea when assigning value. This would also improve things in other scenarios, like if you had a collaborative textarea bound to a value that multiple users could edit simultaneously.The text was updated successfully, but these errors were encountered: