symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
857 stars 315 forks source link

[Live Component] Feature proposals: easily configure norender/filters + prevent form validation #566

Open stefliekens opened 1 year ago

stefliekens commented 1 year ago

Hello,

We are using Symfony\UX\LiveComponent\ComponentWithFormTrait to make the forms in our application "live" to dynamically add or remove fields based on chosen select options (FormEvents::POST_SET_DATA) and handle form validation + submissions via AJAX.

We noticed that when filling a form with approximately 10 fields, a lot of AJAX-requests happen. This is because of the on(change)|* on the form itself. In our use case, we rather want to avoid these unnecessary AJAX calls. We could prevent this re-rendering by doing something with the norender filter like below:

$builder
    ->add('email', EmailType::class, [
        'required' => true,
        'attr' => ['data-model' => 'norender|registration_form[email]'],
    ])
    ->add('firstName', TextType::class, [
        'required' => true,
        'attr' => ['data-model' => 'norender|registration_form[firstName]'],
    ])
     ->add('type', ChoiceType::class, [
        'required' => true,
        'choices' => ['b2c' => 'B2C', 'b2b' => 'B2B'],
    ]);

    $builder->get('type')->addEventListener(
        FormEvents::POST_SET_DATA,
        fn (FormEvent $event) => // Add specific B2B form fields
    );

Note: we accomplished this because we temporary disabled the ux.twig_component.event_listener.data_model_props_subscriber subscriber (see https://github.com/symfony/ux/issues/555).

So, we are looking for a general solution to easily configure this "no-re-render" without specifying and repeating the norender|the-form-name[field-name] for each field. The more complex forms you build (e.g. with children form types) the harder it is to specify the norender filter.

Furthermore, how could we prevent form validation when filling in the form and only run it on the submit (live) action?

Are these 2 feature proposals on the roadmap already or something you guys are interested in?

weaverryan commented 1 year ago

Hi there!

I appreciate you opening the issue from a real-world use case - that's exactly what we need right now :).

I think there are two separate, but related questions. I'll discuss them backwards:

A) How could we prevent form validation when filling in the form and only run it on the submit (live) action?

This should be possible (but please tell me if it's not - I can't remember if we have a test case specifically for it) via:

<form data-model="on(change)|norender|*">

That should trigger the models in your form to "update" on change... but not actually to trigger the re-render. In this case, only live actions would cause the form to re-render. You would trigger this change by overriding the private getDataModelValue() method from ComponentWithFormTrait.

B) So, we are looking for a general solution to easily configure this "no-re-render" without specifying and repeating

This is not yet on the roadmap, but I was wondering when & how this might be requested :). It's quite reasonable, so the trick is finding the best way for this feature to look. The trick is that, on the <form> tag, we need to describe the behavior that we want for ALL of the fields in the form, all at once. The first idea that comes to mind is something like this:

<form on(change)|registration_form[type] on(change)|norender|*>

I'm not sure yet if I love it, and certainly we would need some helper to generate this: the syntax is a LOT. The idea would be that, inside data-model, you specify the behavior you want for any specific fields first (e.g. registration_form[type]). Then, at the end, you have a "catch-all", which fills in the behavior for the remaining fields (in this case you want the norender behavior).

How would this be configured in a real-situation? We might need some mini-builder inside of ComponentWithFormTrait - e.g. something like:

private function configureModelBehavior(ModelBehavior $models)
{
    $models->add('type')
        ->onChange();

    $models->add('*')
        ->onChange()
        ->noRender();
}

I think this would all work... though there may be a simpler / better option that's not clear to me yet.

WDYT?

weaverryan commented 1 year ago

Also, you mentioned:

Note: we accomplished this because we temporary disabled the ux.twig_component.event_listener.data_model_props_subscriber subscriber (see https://github.com/symfony/ux/issues/555).

I couldn't quite see how the subscriber was causing problems - I would love more details inside of #555 - I'd like to solve that issue.

veewee commented 1 year ago

Hello,

I'm coworking on this with Stef and wanted to jump in on the conversation:

A) How could we prevent form validation when filling in the form and only run it on the submit (live) action?

You suggested to

<form data-model="on(change)|norender|*">

This will avoid all rerenders during change. There are however, some fields that we do want to live-update. If we do force one field to rerender, then the validation is applied on all data and the errors will be displayed on the other form fields. What we suggest here, is adding a way to disabling the validation step during data change. (Or maybe even to define validation modes like: early, late, ...)

From a UX point of view, it is probably not the best idea to show errors on fields that haven't been touched yet. You could even argue that doing late validations is almost always better. Some articles:

B) So, we are looking for a general solution to easily configure this "no-re-render" without specifying and repeating

You suggested to:

<form on(change)|registration_form[type] on(change)|norender|*>

What I love about this approach: you decouple the "live" part from the SF form. This makes it possible to reuse form parts with different data binding rules on e.g. different pages.

What I hate about this approach: It is very confusing... It seems hard to debug what rules are applied from your browser developer tools. The helper is nice to build the initial configuration. In most applications, this is the easy part. It becomes harder in maintenance mode where you have to grasp what is going on.

In the form we are building ATM, we are using the SF form 'attrs' to manually set the 'data-model' attribute on the form. this works but has the inversed pros/cons than the once I described above. Maybe we can find out a system that only has the pro's?

I'm thinking in line of the helper you suggested, but having it applied to the form's children instead?

Something like:

private function configureModelBehavior(ModelBehavior $models)
{
    $models->add('type')
        ->onChange();

    $models->add('*')
        ->onChange()
        ->noRender();
}

which resolves in:

<form data-model="on(change)|norender|*">
    <select name="form[type]" id="form[type]" data-model="on(change)|form[type]">
        <option>....</option>
    </select>
</form>

This gives you only the pro's:

WDYT?

I'm planning to play around with the form component trait this week and will let you know our findings!

veewee commented 1 year ago

Small update:

On A), I was thinking to add validation modes: 'late' or 'early' Made a little POC to give you an idea about what I was describing: https://github.com/veewee/ux/pull/1/files

It has some drastical changes in comparison to the original code. I still need to address and tackle these changes to make it more compatible with the original version. (I do not fully grasp the isValidated flag and validationErrors list atm)

The PR splits submit into hydrate and submit. Validation Modes:

By storing wether a form was submitted or not, we could e.g. change the configuration of B) based on submitted or not. For example: you might want to rerender an errorneous field in realtime on the server without hitting the submit button again during late validation.

kbond commented 1 year ago

Just want to pop in to say some kind of builder for complex expressions is interesting. Agree these can get hard to understand.

Would be interesting if you could build these expressions right in your form type:

$builder->add('field', options: [
    LiveOptions::create()->onChange()->noRender(),
]);
weaverryan commented 1 year ago

Hi!

You've proposed 2 cool features.

1) Better field-by-field model configuration

For forms, we currently allow you to do <form data-model="*"> (or some similar variation) so that you can be lazy and use the name attribute instead of repeating data-model on every field. But thanks to the final HTML code you listed @veewee in your previous comment https://github.com/symfony/ux/issues/566#issuecomment-1330261079 - I'm starting to think that we should NOT allow for this shortcut. Instead, why not (A) force data-model to be on every field but (B) make it really easy to do so, using the builder-like system you proposed. Something like this (I'm taking your syntax and iterating on it a bit):

private function configureModelBehavior(ModelBehavior $models)
{
    $models->field('type')
        ->onChange();

    $models->default()
        ->onChange()
        ->noRender();
}

This would generate something like:

<form>
    <select name="form[type]" data-model="on(change)|form[type]">
        <option>....</option>
    </select>

    <input name="form[firstName]" data-model="on(change)|norender|form[firstName]">
</form>

This would allow us to remove the <form data-model functionality entirely... but life is even easier for the user. I think it's a win-win? I think we could accomplish this by "walking through" the FormView just before rendering to add all of the attributes to each field.

2) Validation Modes

This is a very cool idea, and a very good use-case where you want to re-render some fields but not apply validation "early" - thanks for sharing that.

So, I'm 👍 for this. I like the idea of having early validation or late validation. About your PR:

It has some drastical changes in comparison to the original code. I still need to address and tackle these changes to make it more compatible with the original version. (I do not fully grasp the isValidated flag and validationErrors list atm)

By validationErrors, I think you meant validatedFields. I'll assume you did for the explanation below :).

Currently, there are 2 ways that a form component might show validation:

A) real-time/early & partial validation When the user changes a field, on JavaScript, we add that model name to the validatedFields array. There's actually some special code for this - https://github.com/symfony/ux/blob/2.x/src/LiveComponent/assets/src/Component/plugins/ValidatedFieldsPlugin.ts#L12-L20

Suppose the user modifies a firstName field, a lastName field and then something triggers a re-render. In this case, validatedFields will contain firstName and lastName. We then only show those 2 errors (and hide any errors for fields that have not been modified yet). The validatedFields property is a LiveProp so that it persists through re-renders. This is necessary so that, if the user THEN modifies the email field and triggers a re-render, the Ajax request will now send validatedFields set to [firstName, lastName, email]: it will "remember" which fields were previously modified/validated + now also validate email. This is how the "early" validation is smart enough to only show errors next to the fields that the user has actually modified.

B) Full form validation In the above scenario, the user might eventually click the "submit" button to trigger a LiveAction. In your LiveAction, you will call $this->submitForm(). The optional first arg to submitForm is bool $validateAll = true. This tells the system to validate ALL the fields, regardless of what is inside of validatedFields. To do this, we set the isValidated flag. This is also a LiveProp so that every render afterwards remembers to validate everything. You've renamed this to $wasSubmitted in your PR (which might be a better name), but I think you're trying to accomplish the same thing with it. The previous implementation of how this flag was set may have worked fine for your use-case, but I could be wrong :).

Also, about this:

By storing wether a form was submitted or not, we could e.g. change the configuration of B) based on submitted or not. For example: you might want to rerender an errorneous field in realtime on the server without hitting the submit button again during late validation.

That's really cool! We can worry about that later, but yes, it could be an option when you're configuring the model behavior - e.g.

private function configureModelBehavior(ModelBehavior $models)
{
    $models->field('type')
        ->onChange();

    $models->default()
        ->onChange()
        // the double-negation is confusing here, so that would need to be improved
        ->noRender(!$this->isFormSubmitted());
        // other possible flavor
        ->skipRenderingOnUpdateUnless($this->isFormSubmitted())
}

@kbond

Would be interesting if you could build these expressions right in your form type:

That indeed might be interesting, though in this moment, I'm sticking with putting all of this logic into the form trait instead of the form class, though it may make sense to also allow it in the form class... it's not clear yet. One weird thing about the form class (other than coupling form to live component, if you care) is that you could configure the attributes... but then if you don't render the form inside a component... it won't work anyways.

veewee commented 1 year ago

@weaverryan Completely agree with your previous comment. This would be awesome! Let me know if there is anything else I can do to make this possible.

Note: I indeed plaid with the validation properties, but nothing seemed to give good results. The flags are conditionally changed in the trait so are not very reliable ATM.

veewee commented 1 year ago

Small update from our side: I've updated the POC to work with a simple version of the model behaviour See https://github.com/veewee/ux/pull/1/files

It can be used this way:

    protected function configureModelBehavior(ModelBehavior $models): void
    {
        $models->default($this->wasSubmitted ? 'on(change)' : 'on(change)|norender');
        $models->field('registration_form[type]', 'on(change)');
    }

This works pretty good! We're gonna be using this version in our project until this package provides the functionality described above.

weaverryan commented 1 year ago

Btw,

$models->field('registration_form[type]', 'on(change)');

Did you think about allowing just this?

$models->field('type', 'on(change)');

That's what I had in mind (not needing the form name), but I wanted to make sure you didn't see this as a problem. I think it should be possible - we already, iirc, grab the form "name" in the trait for some different reason.

veewee commented 1 year ago

I went with this approach because we have nested forms. That way you could do form[subform][child]. This was the easiest solution, because form[$fieldName] results in form[subform[child]] in that case

weaverryan commented 1 year ago

Hmm, ok, will need to keep this sub-form case in mind. Ideally we can implement this without needing the registration_form[] prefix :). thx

carsonbot commented 7 months ago

Thank you for this suggestion. There has not been a lot of activity here for a while. Would you still like to see this feature?

carsonbot commented 6 months ago

Could I get a reply or should I close this?

carsonbot commented 6 months ago

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

TdWa commented 1 month ago

Would still be great to be able to disable automatic form submission and validation, without disabling rerenders. For now the best workaround I found is to hide the validation errors until the user manually submits the form.

smnandre commented 1 month ago

Would you be up for a PR ?

TdWa commented 1 month ago

Would you be up for a PR ?

I'm afraid I don't have enough experience yet with Symfony UX or frameworks in general to contribute at the moment

TdWa commented 1 month ago

I think it is caused by this? https://github.com/symfony/ux/blob/33f637d1660125a0c7c6b2eb5062af1f7c537788/src/LiveComponent/src/ComponentWithFormTrait.php#L63

Maybe it's a relatively small change to add an option to make it false

YummYume commented 3 weeks ago

Preventing validation on re-renders is pretty straight-forward : keep the validatedFields array empty. This is the trick I am using right now to prevent this behavior :

protected function instantiateForm(): FormInterface
{
    // This will always keep validations empty unless $this->submitForm() is called, or validations are done manually
    $this->validatedFields = [];

    return $this->createForm(...);
}

It's certainly a hack, some may even call it dirty, but it does the job in the meantime.