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

Feature/form change event #281

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

james2doyle
Copy link

I am building a form builder that allows users to connect actions to inputs. Without the form emitting its change events, there is no way for me to react to updates in the form without relying on native DOM event listeners. Not ideal.

This PR just adds onChange to the top level form element so when any of the children fire change the top level form can emit those changes.

@justin-schroeder
Copy link
Member

Hi @james2doyle!

Thanks for this PR, I'm not opposed to adding this in, and thank you for the idea. I am wondering if you could help me understand how this event is beneficial/different from the existing input event. Also, can you confirm this event is emitted when the form is mutated by manipulating a v-model?

@james2doyle
Copy link
Author

Good point. I will check that.

I'm actually using a schema to build my form. From what I can see in the docs, there isnt a way to add event listeners via the schema object. Am I mistaken?

By listening for form change at the top level, I can listen to all the children without extra handlers, and also capture dynamic inputs as well.

@justin-schroeder
Copy link
Member

@james2doyle correct, currently there is no way to bind events to inputs using schemas. Although thats a high priority feature in v2.5. In the meantime you could also use @gahabeen's vue-formulate-extended plugin

The @input event should bubble "change"-like events all the way at the top of the form too since it's the event behind v-model, but I would be really interested in learning about how the two events differ?

<FormulateForm
  :schema="yourSchema"
  @input="inputHandler"
/>

Additionally, I'm pretty sure you can already bind the change event by using the .native vue modifier:

<FormulateForm
  @change.native="changeHandler"
/>

If there's a clear use case for one over the other then by all means (!), let’s get this merged — but if they functionally are the same or nearly the same, I'd rather just stick with one event so people don’t have competing concepts they need to learn.

@james2doyle
Copy link
Author

Ah ok. Thanks for the link! I am also using a render function so I'm doing lots of quirky things. Let me look into it and share an update when I find something

@james2doyle
Copy link
Author

Hmm there is some weird stuff going on here: https://codesandbox.io/s/vue-formulate-reproduction-template-forked-65lsp?file=/src/components/Reproduction.vue

You can see that none of the changes fire on input of the text field but the native one does seem to fire when you blur the text field after input. You may also notice it seems that the input event is actually firing twice on each input of the text field.

I should also note that the change I emitted from the form in this PR passes down the FormSubmission like a submit does. So it isnt quite equivalent to the native change event

@james2doyle
Copy link
Author

james2doyle commented Oct 8, 2020

Ok I think I may have found a bug. Seems like when input is added to the top level of the form tag, you get way more events that actually occurred. I added a failing test for this. Seems like it is triggering on mount, then triggering twice on an actual input. So the event is firing 3 times when I would only expect it to fire once

@james2doyle
Copy link
Author

Any thoughts on this? I think this is still a bug as the test shows

@justin-schroeder
Copy link
Member

Yeah i agree. it looks kind of like a bug. Sometimes double events on the form are unavoidable byproducts of the input's model and the form's model syncing, but obviously this looks like it needs to be cleaned up. Feel free to look for a solution to it — probably makes sense to file a separate bug report issue too. Thanks!

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

Successfully merging this pull request may close these issues.

2 participants