jonjamz / blaze-forms

Dead easy reactive forms with validation (Meteor).
https://atmospherejs.com/templates/forms
MIT License
113 stars 11 forks source link

Regression: keyup event vanishes #40

Closed steph643 closed 9 years ago

steph643 commented 9 years ago

Might be related to https://github.com/meteortemplates/forms/issues/37.

This does not work anymore:

    'click #randomColors': function(event, template)
        {
        template.$('[name="bkgColor1"]').val(randomColor()).trigger('keyup');
        template.$('[name="bkgColor2"]').val(randomColor()).trigger('keyup');
        template.$('[name="textColor"]').val(randomColor()).trigger('keyup');
        }

Only the first trigger('keyup') runs correctly, next ones do not validate the fields.

jonjamz commented 9 years ago

Ah yes. I'll have to make throttling optional on elements.

steph643 commented 9 years ago

Aren't you suppose to have a different throttling function for each element?

I would perfectly understand that this doesn't work (second event on same element):

template.$('[name="bkgColor1"]').val(randomColor()).trigger('keyup');
template.$('[name="bkgColor1"]').val(randomColor()).trigger('keyup');

But I don't understand why this doesn't work (second event on a different element):

template.$('[name="bkgColor1"]').val(randomColor()).trigger('keyup');
template.$('[name="bkgColor2"]').val(randomColor()).trigger('keyup');
jonjamz commented 9 years ago

Yeah I think I might need to create the throttled function within each template instance to avoid this.

jonjamz commented 9 years ago

I realized what the problem is though--I have a reactive {{value}} on the input, and sometimes the time it takes for a new typed value to go through validation and then back into the DOM glitches out the input as I'm typing. When I removed the reactivity with Tracker.nonreactive it fixed the problem. So now I'm trying to figure out what to do here--either throttle it somewhere else and see how that works, or keep the value helper non-reactive.

jonjamz commented 9 years ago

Ok I'm going to revert #37 and remove throttling.

jonjamz commented 9 years ago

I found a better solution:

reactiveValue: ->
  inst = Template.instance()
  component = inst[MODULE_NAMESPACE]
  return component.value.get()

value: ->
  inst = Template.instance()
  component = inst[MODULE_NAMESPACE]
  return Tracker.nonreactive -> return component.value.get()
ReactiveForms.createElement
  template: 'inputElement'
  validationEvent: 'keyup'
  created: ->
    this.focused = new ReactiveVar(false)

Template.inputElement.helpers
  focused: ->
    inst = Template.instance()
    return inst.focused.get()

Template.inputElement.events
  'focus .reactive-element': ->
    inst = Template.instance()
    inst.focused.set(true)
  'blur .reactive-element': ->
    inst = Template.instance()
    inst.focused.set(false)
<template name="inputElement">
  <input type="text" class="reactive-element" value={{#if focused}}{{value}}{{else}}{{reactiveValue}}{/if}}>
</template>
steph643 commented 9 years ago

Sounds good.

steph643 commented 9 years ago

I believe {{value}} is what you need in most cases. For example, in your inputElement above, I am not sure you really want your fields to be reactive and flicker while you are filling the form. However, having {{reactiveValue}} provides great flexibility depending on your need. But then I have a question: the changed argument of the action function is computed based on {{value}} or {{reactiveValue}}?

jonjamz commented 9 years ago

After passing validation, each element's data is stored in a reactive property in that element's instance, but then it's also passed up to the form block if possible, where it's stored in a non-reactive object. So the action function receives that object--but you can't submit the form unless all the values pass validation when you're using simple-schema.

steph643 commented 9 years ago

Here is what frightens me:

Use case 1: 1- I have a form with reactive values, modified distantly by other users (I can see changes on the screen). 2- I don't change anything and click submit. 3- changed should contain nothing, because there is nothing to update in the database compared to what I see on the screen.

Use case 2: 1- I have a form with non reactive values, modified distantly by other users (I can't see their changes). 2- I don't change anything and click submit. 3- changed should contain all fields that have been modified by others so that I can update the database with the values I see on the screen when pressing submit.

Scary? Time to remove changed altogether?

jonjamz commented 9 years ago

I'm actually working on this right now. I don't think the elements currently respond to reactive changes in the underlying data, but I'd like them to.

For the changed state, I think it should only run if you made a local change. Remote changes that affect the underlying data are already saved (hence how they get published to you). The changed state should be true only when you change a value in the form, and perhaps the changed argument in the action function could perform a diff on submit, when it detects that the underlying data has been changed during your form session.

Here's an interesting case--someone remotely changes the underlying data to something invalid. The form won't let you submit until you change it to something valid.