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

Text disappears while typing #343

Closed
mstenta opened this issue Apr 22, 2020 · 7 comments · Fixed by #390
Closed

Text disappears while typing #343

mstenta opened this issue Apr 22, 2020 · 7 comments · Fixed by #390
Labels

Comments

@mstenta
Copy link
Member

mstenta commented Apr 22, 2020

I use Android's "glide type" feature for typing into text fields on my phone (spell out a word by dragging my finger over all the letters). I'm finding that this is buggy in Field Kit. After "glide typing" a word, it sort of flashes and then disappears, and I have to do it again. It is pretty consistent, but sometimes the word does stick.

I have to assume this is something to do with the way Vue.js is monitoring/propagating changes to the input fields, but I don't know enough to be able to test further. I tried testing the same glide typing in this example Vue.js form and did NOT experience the same issue: https://vuejsexamples.com/simple-vue-js-contact-form-2/

@mstenta mstenta added the bug label Apr 22, 2020
@jgaehring
Copy link
Member

This is weird. I've looked around and experimented with things and am getting wildly erratic results; sometimes reproducing this error, sometimes not.

I've found some similar issues reported with Vue's v-model="foo" directive, which recommend using :value="foo" @input="foo = $event.target.value" instead (v-model is essentially just syntactic sugar for that), but we're already using the syntax for all our forms, since we're using Vuex for log data instead of component state. Another caveat from Vue's docs on Form Input Bindings:

For languages that require an IME (Chinese, Japanese, Korean, etc.), you’ll notice that v-model doesn’t get updated during IME composition. If you want to cater to these updates as well, use the input event instead.

But again, this amounts to essentially the same advice as above. Soooo... 🤷‍♂️

Afraid to say I'm gonna punt on this one though until after 1.0.0 or later.

@mstenta
Copy link
Member Author

mstenta commented May 20, 2020

Hmm now i'm seeing the same behavior in my browser on my laptop as I'm typing on my regular keyboard. :-(

So this might not just be an Android glide typing issue.

@mstenta mstenta changed the title Text disappears when entered via Android glide typing Text disappears while typing May 20, 2020
@mstenta mstenta added this to the v1.0.0 milestone May 27, 2020
@mstenta
Copy link
Member Author

mstenta commented May 27, 2020

Bumping this up as a blocker to v1.0.0 because it makes the text fields almost unusable. I'm curious if I'm the only one experiencing this. It's happening consistently in Firefox on Android and Ubuntu.

@jgaehring
Copy link
Member

Hm, I'm still having trouble reproducing this consistently. I can't even reproduce it with glide typing now. I've tried it on Android Chrome (with multiple keyboards), Ubuntu Chrome and Ubuntu Firefox. No issues.

Can you describe the behavior as you're experiencing it on your laptop with a physical keyboard?

Have you looked to see if there are any errors appearing in the console on Ubuntu Firefox?

@jgaehring
Copy link
Member

Ok, after a call with @mstenta, I think we've identified some likely causes.

Initially, I thought the main culprit was farmLog.updateLog, because it's a computationally heavy operation. Thinking about this more, that may certainly be exacerbating the problem (and could be why this problem has only surfaced now), but I think the heart of the problem is too many asynchronous function calls being used to propagate changes through the Vuex store, with no means of retaining the original sequence of the input events.

It seems very likely to me that there are race conditions between the initial input event, the updateLog Vuex action, the updateCachedLog action and the final DOM rendering. The input event and updateCachedLog action are very obviously asynchronous, but critically, so is updateLog, since all Vuex actions are asynchronous (ie, they always return a promise).

I'm oversimplifying, but we essentially have 4 steps for that must occur each time a log updates:

  1. The EditLog component's updateCurrentLog method dispatches the updateLog action with the log's localID and its new properties.
  2. In the Vuex store, the updateLog action asynchronously looks up the log in the store by its localID, then with that reference to the log's current state and the new properties, makes an updated copy using farmLog.updateLog, setting both isCachedLocally and wasPushedToServer to false. It then commits the the copy to the store, using the addLogs mutation.
  3. Whenever addLogs is committed on a log with isCachedLocally set to false, that triggers a subscriber that calls updateCachedLog
  4. updateCachedLog makes an asynchronous call to the db, and whenever that finishes, it will commit addLogs again, this time with the same updated log but with its isCachedLocally property to true.

So let's say we have 2 successive updates from the input field, first Update A then Update B, and they're updating the name field, which prior to Update A was just "t"; Update A gets an input value of "th"; Update B gets an input value of "the". The sequence of events is intended to look like this:

Update Step log in Vuex log in IndexedDB
A 1 { name: "t", isCachedLocally: true } { name: "t" }
A 2 { name: "t", isCachedLocally: false } { name: "t" }
A 3 { name: "th", isCachedLocally: false } { name: "t" }
A 4 { name: "th", isCachedLocally: true } { name: "th" }
B 1 { name: "th", isCachedLocally: true } { name: "th" }
B 2 { name: "th", isCachedLocally: false } { name: "th" }
B 3 { name: "the", isCachedLocally: false } { name: "th" }
B 4 { name: "the", isCachedLocally: true } { name: "the" }

However, it may actually be executing in this sequence:

Update Step log in Vuex log in IndexedDB
A 1 { name: "t", isCachedLocally: true } { name: "t" }
A 2 { name: "t", isCachedLocally: false } { name: "t" }
B 1 { name: "t", isCachedLocally: true } { name: "t" }
B 2 { name: "t", isCachedLocally: false } { name: "t" }
A 3 { name: "th", isCachedLocally: false } { name: "t" }
A 4 { name: "th", isCachedLocally: true } { name: "th" }
B 3 { name: "te", isCachedLocally: false } { name: "th" }
B 4 { name: "te", isCachedLocally: true } { name: "te" }

There are a few places where asynchonisity could be biting us in the butt; step 4 is the most conspicuous, but I think step 2 might be even more consequential, because once the updateLog action is called it essentially forks execution down separate, asynchronous paths. So if the second input event is fired off quickly enough, it will reach step 2 before the first update has reached step 3, which is why we end up with Update B producing the state "te" instead of the expected "the".

One thing @mstenta and I noticed was that this bug was easiest to reproduce if you refreshed the "Edit Log" screen and immediately started typing in the name field, particularly while network requests were still running in the background. My first thought was that there was some sort of memory issue, and that the repeated execution of farmLog.updateLog in combination with those network requests were overburdening memory. I realize now, however, that the behavior had much more to do with numerous asynchronous calls flooding the event loop all at once, causing even greater latency between input events and the eventual DOM rendering, which ultimately enabled the race conditions. The same behavior can be observed if you just type extremely fast.

I'm still not 100% sure the best way to remedy this, but I think I've got a better handle on why this is happening now.

@jgaehring
Copy link
Member

jgaehring commented Aug 6, 2020

Some next steps towards resolving this:

  • Move logTypes from shellModule to farmModule, so it's part of the state available from the addLogs mutation.
  • Move logic from the updateLog action into the addLogs mutations, so it's all synchronous.
  • Remove isCachedLocally metadata from logs (see Logs API: Caching #385).
  • Make farmLog.updateLog a little faster by only iterating over the keys provided in props.
  • Make updating logs a lot faster by using getters, setters and symbols instead of updateLog. Improving farmLog with Symbols and Metaprogramming #358

@jgaehring
Copy link
Member

Move logTypes from shellModule to farmModule, so it's part of the state available from the addLogs mutation.

In the course of this I'm totally overhauling how the logTypes are updated from the server and cached locally, separating it out from the updateSiteAndUserInfo and loadCachedUserAndSiteInfo actions. For one thing, I'm storing everything as resources, instead of just stripping out the logTypes and discarding other information for assets, areas, etc. I also want to implement some smarter checks on exactly how those resources get updated, but I'm torn how to do so. For the time being, I'm just using Ramda's mergeDeepRight to merge the old resources with the new, but I'm wondering if this could be prone to errors. I'm trying to think what is the best way to handle changes to the server configuration of log types so that those changes will be reflected in FK, while also making sure we have a way to migrate logs that might be effected by those changes. I'm struggling to understand how and why those changes might occur, so it might be good to have a talk with @mstenta to figure out how best to accommodate such changes.

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

Successfully merging a pull request may close this issue.

2 participants