-
-
Notifications
You must be signed in to change notification settings - Fork 146
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
Input field reverts to old value while db updates #219
Comments
This problem has been bugging me for months now, so I spent last night debugging and playing around with https://github.com/day8/re-com/blob/master/src/re_com/misc.cljs#L127 The thing with re-frame events is that they are dispatched asynchronously -- they will be handled (very) soon, but not right now. By resetting Let's replay what happens, in slow motion:
Transition from 3 to 4 to 5 causes that oscillation we see. Omitting external-model resets I pointed out seems to work just fine - in step 3, we avoid the component update, dispatch a command to update the outside wold, and once the world updates, it will let us know by a subscription change. Only then the component updates its internal state I may be missing something though, so before cooking up a pull request, I'd welcome some feedback -- would removing them resets break something I don't see? I'm rudely pinging @Gregg8, since he seems to be the last one to touch the code in question (although it's been 6 years). |
Upon further inspection, the resets are there for a reason. Hm... Edit: The reasons do not hold, imho, see the mention below. |
Great that someone else is bothered by this! :-) Have you been able to fix this bug while preserving the bugfix in #187? If not, I have been thinking about this and my thought is that there may be an underlying problem, namely when does the input change from showing the (edited) internal value to the (persisted) external value from app-db?
Does that make any sense? Or is there a reliable way for the input to detect when "very soon" has happened? |
Well, I think there was no bug to fix in #187 in re-com, the problem was in the example code itself. It was twisted and wrong, and I've explained it there and sketched out a more fitting solution. I've implemented a text input component that uses external/internal model mechanism, it works exactly like the re-com one, minus the resets. It works just fine, it updates itself whenever the external model changes (given the model actually changes), and is flicker-free when sending changes to the outer world via on-change. |
Wow, great! Are you able to share the source for that component? |
Just copy and paste the re-com one, and delete those three resets I've pointed out in my first comment here. :) |
Thanks @brjann for the bug report and @prook for the subsequent investigation. I've tested the proposed change locally with a few different options; e.g. I agree its a reasonable trade-off since the example in #187 can be written in the way suggested by #187 (comment) vs it is harder to work around the issue described here without a change to the component itself. |
It looks like this fix leads to the problem that I predicted above, that if the change is rejected by the DB, the text in the input still does not change. So if I change the text to some illegal value and the dispatched event simply reverts the value back to its old value (i.e., does not update app-db), the value does not change in the text field. I've updated my demo of the bug to reflect this https://github.com/brjann/re-frame-test |
Thanks for reopening @superstructor! I have an idea on how it could be solved, but it feels very hackish. After the on-blur function is called, a "dummy event" is dispatched with the only purpose of resetting the subscription to the db / external value. The idea is that the dummy event will be handled after the event that is supposedly dispatched in the on-blur function. So then it should be safe to rely on the db / external value. |
But why should it change? To what should it change? To the original app-db value, throwing the user input away? That would be bad, wouldn't it? When the input is invalid, the app should ask the user to fix it, not toss it away and make them start over. :) |
Hm, this is a tricky situation. You are right that if the value is reverted, the app should inform the user that something is wrong and not necessarily throw the value away. Is there no conceivable situation where the app should just "silently" revert the value? |
At the risk of sounding arrogant, I'm afraid this discussion has little to do with the original issue. However, the snippet in #187 (comment) should provide an idea how to trigger a reset/re-render of a component using signal other than model change. |
If the input no longer reflecting the value of the subscription can be considered an unwanted effect of the fix, is not the discussion relevant? |
Regardless, I can accept that the edited value doesn't change if the edit is reverted, since the app should handle that situation. So I'm fine with closing the issue again @superstructor. Thanks for the fix @prook! |
Thanks @brjann and @prook for your discussion and input on this issue. Myself and @Gregg8 were not quite ready to give up so I re-opened (again) and we found a solution @brjann First, let us define the problems. Problem 1: "Invalid Value Displayed"If the user types a value that is subsequently modified in Problem 2: "Flicker Between Values"The fix to Problem 1 causes this problem. If there is a delay in processing The root cause here is essentially we are forcing a render too early before Solution
This means that Input text detects the number of arguments received by on-change so you only need to call |
Also posted in Clojureverse https://clojureverse.org/t/input-field-blinks-before-update-in-re-frame/6758
I'm using the
re-com/input-text
element. The value is dispatched to the db when the input loses focus, which is the default behavior. However, after leaving the input field, the old value is shown for a short time, and then the new value is shown again.Basically, the code looks like this
Can I prevent this behavior somehow? I've made a quick demonstration of the phenomenon based on the re-frame Leiningen template available at https://github.com/brjann/re-frame-test
The text was updated successfully, but these errors were encountered: