day8 / re-frame

A ClojureScript framework for building user interfaces, leveraging React
http://day8.github.io/re-frame/
MIT License
5.43k stars 715 forks source link

Editing input fields #39

Closed smahood closed 9 years ago

smahood commented 9 years ago

When running the simple example, if the color textbox is edited anywhere before the end of the input, when the input is changed it will reset the cursor to the end of the input box.

Any idea how to fix this? I used it as an example for how to save form input and am running into the same problem in my own app as well.

hura commented 9 years ago

Common "problem" with React:

http://stackoverflow.com/questions/28922275/in-reactjs-why-does-setstate-behave-differently-when-called-synchronously

You'll find lots of github issues at Reagent/OM and others with the exact same problem. The only fix is to use .setState(). Though, I'd probably recommend using uncontrolled inputs:

https://facebook.github.io/react/docs/forms.html

Changing it do :default-value should fix it and keep everything else working. This is a good solution if you do not need to do any filtering/transformation for the input (for instance, only allow numbers or make all input uppercase). Otherwise you'll have to mess with some lower level .setState stuff.

smahood commented 9 years ago

Thanks for that @hura, that will solve my immediate problem.

Could I recommend that the simple example be changed to the same uncontrolled input behaviour, and perhaps a future example deal with a more complex controlled input example?

hura commented 9 years ago

Yes, I think the example should be fixed. You could do a pull request.

IMHO a more complex example should go to reagent since it's not really re-frame specific. I'd say the reagent wiki would be a good place if you find out a nice way to do this.

mike-thompson-day8 commented 9 years ago

If anyone is writing this up, in addition to @hura's links here is more background: https://github.com/reagent-project/reagent/issues/79

mike-thompson-day8 commented 9 years ago

@whodidthis made this observation via the news group:

Probably something like when you type quickly letters a b c d and they are handled by re-frame sequentially with view updated every requestAnimationFrame (16ms?) so the following happens:

type a dispatch a view a type b dispatch ab type c dispatch abc view ab type d dispatch abd view abc view abd

So input now has value abd even when you typed abcd. But it's not just loss of data, it also messes up ctrl+z undo and other stuff

We tend to use onblur as our update trigger (not char by char) and so we don't run into this problem. So this is not an itch I personally need to scratch (other larger ones are top of mind). So, for the moment, I'll be leaving it to those affected to form/propose a solution.

smahood commented 9 years ago

Yeah, I'm sort of forming the opinion that re-frame isn't a really good fit for the char by char view updates, but I hope char by char app-db updates will work. I'm going to try and work on some simple ideas for the simple example and hopefully figure something good out that fits into the re-frame conceptual model, then do some work on a pull request for the example and wiki.

yatesco commented 9 years ago

Shaun, have you seen the related conversation (and proposed workaround) in the thread "re-frame/react - controlled input losing key presses"?

On 5 April 2015 at 03:29, Shaun Mahood notifications@github.com wrote:

Yeah, I'm sort of forming the opinion that re-frame isn't a really good fit for the char by char view updates, but I hope char by char app-db updates will work. I'm going to try and work on some simple ideas for the simple example and hopefully figure something good out that fits into the re-frame conceptual model, then do some work on a pull request for the example and wiki.

— Reply to this email directly or view it on GitHub https://github.com/Day8/re-frame/issues/39#issuecomment-89706826.

simax commented 9 years ago

I would love to see some examples on the wiki on how we deal with input. I've been getting very frustrated (lots of it to do with my own lack of understanding :)). I initially tried to use :value and thought everything was fine until I noticed the problem described in this very issue. Then after some googling I went for :default-value which was ok until I noticed that if I fetched another set of values, then any inputs where I used default-value where not getting updated (although the render function was being re-run). Then after more googling I went for :default-value with ^{:key default-value}. That seems to work but I've been on this for days. Now I use this generic function shown below for my input boxes. So far it works with HTML5 text, date, email and number.

(defn input-element [{:keys [id name type placeholder on-blur default-value]}]
  "An input element which updates its value on change"
  ^{:key default-value} [:input
                         {:id id
                          :name name
                          :placeholder placeholder
                          :class "form-control"
                          :type type
                          :default-value default-value
                          :on-blur on-blur
                          }])
smahood commented 9 years ago

Thanks @yatesco and @simax. Using ^{:key} with default-value and on-blur seems like the best bet so far for handling general inputs. I'm going to try out some other options and get something written up on the wiki.

smahood commented 9 years ago

So I was able to get a form-3 component with char by char updates mostly working. It mostly fixes the problem of resetting the cursor position, but there is still the issue of lag and missing keystrokes when typing quickly.

I've put up a repo with some different methods of controlled inputs at https://github.com/smahood/re-frame-inputs

It's a bit ugly, but the examples with the form-3 components seem like they have some promise for true char by char updates. If anyone wants to look at it, my next thought is to offload some of the synchronization between the components and app-db into the component local state so that the input lag is moved from the ui display to the speed of updating the app-db. Not sure how yet though.

My thought is to use the repo to experiment and then use the results to write up some patterns in the wiki. Feel free to poke around in there and suggest improvements or point out mistakes.

whodidthis commented 9 years ago

re-frame.core/dispatch-sync and the usual handler behaves like .setState, you can use that

smahood commented 9 years ago

Well, that is way easier. Is there anywhere that it will cause problems? I was reading https://groups.google.com/forum/#!topic/clojurescript/_XvbwEVYVUc and just assumed it was more trouble than it was worth, but maybe this is just the situation it is meant for.

@mike-thompson-day8 if you can confirm that calling dispatch-sync in the on-change handler for text inputs is a safe default, then I will update the simple-example and do a pr for it as well as write up the first draft of a wiki page for handling inputs.

whodidthis commented 9 years ago

Actually i was the one being a bad and spreading some doubts about dispatch-sync. But that was just me being a bad, dispatch-sync is perfect.

React puts in a lot of work to make focus work while updating form elements, synchronous changes are the only way it works.

mike-thompson-day8 commented 9 years ago

I was wondering how to complete this ticket:

yatesco commented 9 years ago

I would update the simple example, add an FAQ and even update the front page as the current convention of dispatch rather than dispatch-sync to update controlled input forms is unusable I think.

On 21 April 2015 at 14:00, mike-thompson-day8 notifications@github.com wrote:

I was wondering how to complete this ticket:

  • is there consensus about the correct approach?
  • should simple example be changed? If so how?
  • should we create an FAQ entry?
  • anything else?

— Reply to this email directly or view it on GitHub https://github.com/Day8/re-frame/issues/39#issuecomment-94783519.

yatesco commented 9 years ago

Happy to do this after next week?

On 21 April 2015 at 14:03, Colin Yates colin.yates@gmail.com wrote:

I would update the simple example, add an FAQ and even update the front page as the current convention of dispatch rather than dispatch-sync to update controlled input forms is unusable I think.

On 21 April 2015 at 14:00, mike-thompson-day8 notifications@github.com wrote:

I was wondering how to complete this ticket:

  • is there consensus about the correct approach?
  • should simple example be changed? If so how?
  • should we create an FAQ entry?
  • anything else?

— Reply to this email directly or view it on GitHub https://github.com/Day8/re-frame/issues/39#issuecomment-94783519.

mike-thompson-day8 commented 9 years ago

The new router loop solves this problem. Because it no longer uses core.async, the delays are gone. See 420e42aacccbac2d81fedc5ff861442a4ce70c1d Will be a part of v0.5.0 release.