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

Editing input fields #39

Closed
smahood opened this issue Mar 31, 2015 · 17 comments
Closed

Editing input fields #39

smahood opened this issue Mar 31, 2015 · 17 comments
Labels

Comments

@smahood
Copy link
Contributor

smahood commented Mar 31, 2015

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
Copy link

hura commented Mar 31, 2015

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
Copy link
Contributor Author

smahood commented Mar 31, 2015

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
Copy link

hura commented Mar 31, 2015

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
Copy link
Contributor

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

@mike-thompson-day8
Copy link
Contributor

@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
Copy link
Contributor Author

smahood commented Apr 5, 2015

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
Copy link
Contributor

yatesco commented Apr 5, 2015

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
#39 (comment).

@simax
Copy link

simax commented Apr 5, 2015

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
Copy link
Contributor Author

smahood commented Apr 7, 2015

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
Copy link
Contributor Author

smahood commented Apr 7, 2015

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
Copy link

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

@smahood
Copy link
Contributor Author

smahood commented Apr 7, 2015

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
Copy link

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
Copy link
Contributor

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? If so, can someone closer to this add it?
  • anything else?

@yatesco
Copy link
Contributor

yatesco commented Apr 21, 2015

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
#39 (comment).

@yatesco
Copy link
Contributor

yatesco commented Apr 21, 2015

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
#39 (comment).

mike-thompson-day8 added a commit that referenced this issue Jul 10, 2015
Replace dispatch with dispatch-sync for issue #39
@danielcompton danielcompton changed the title Issue in simple example Editing input fields Sep 12, 2015
@mike-thompson-day8
Copy link
Contributor

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

ricardojmendez added a commit to ricardojmendez/memento that referenced this issue Oct 3, 2017
Decided that instead of binding a component against an atom, I'd just
send a notification on change. That way I can keep all state in one place
and clear it if necessary, something that becomes ugly when the state for
the textarea is kept on an atom.

Mike Thompson suggested doing it on-blur instead of updating every
on-change (because of day8/re-frame#39). While
dispatching on-blur is cleaner, I can't update the textarea to change or
clean it unless I use the `value` attribute... which is blank until we
exit it.  Looks like I have to use `on-change`, so the event needs to be
fired with dispatch-sync.
Kauko added a commit to CSCfi/rems that referenced this issue Dec 12, 2018
re-frame seems to have trouble with controlled vs uncontrolled input elements.
Looks like using :default-value helps though:
day8/re-frame#39 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants