[Live] Re-extracting form data after submit #254

weaverryan · 2022-01-28T19:57:54Z

Q	A
Bug fix?	yes
New feature?	yes
Tickets	Addresses part of #221
License	MIT

This fixes 3 bugs at once:

This recalculates formValues after submit, in case the submitted data changes the form's data or underlying structure (e.g. CollectionType)
If a field is suddenly missing entirely from a form (hint: think removing an embedded CollectionType form), then previously validatedFields did not "forget" that field. This caused edge-case bug when removing an embedded form that was validated, then adding a new form a moment later (which would be empty, but now validated).
When passing a FormView into your component that was already validated, on the first re-render, the validation would be lost and only modified fields would be validated.

Ping @Lustmored and @norkunas. This mostly addresses @norkunas's problems with how getFormValues() works, but this was also causing trouble for @Lustmored, potentially for different reasons. So, this may not solve the problems addressed in #221, but it's certainly related.

Cheers!

norkunas · 2022-01-28T20:13:27Z

So if I'll have hundred live actions I'll need to write same number submit/reset statements? :)

weaverryan · 2022-01-29T01:52:34Z

So if I'll have hundred live actions I'll need to write same number submit/reset statements? :)

Yup! The same as if you has a hundred controller actions 😉

BUT, I'm re-thinking this approach - I think it's flawed, as trying to "submit the form" then "change the data the form was created from" then... re-recreate the form with the new mix of data... just doesn't work, as the final "new" form wouldn't be "submitted", so it wouldn't have correct validation errors. If we re-submit the 2nd form, then we overwrite the new values we just changed in our underlying object. I think it's a dead approach.

I think the easier flow is to, in your actions, manipulate $formValues directly. This, I believe, is actually what you were doing all along @norkunas - I was trying to do something fancier. It doesn't look as neat as manipulating the underlying form data DTO/entity, but it's much simpler. The flow in an action would be what you have currently @norkunas (I'm just arriving to the solution later). I still need to try it, and see if my fix for getFormValues() still works with your approach. Will check into it next week.

norkunas · 2022-01-29T06:42:28Z

just doesn't work, as the final "new" form wouldn't be "submitted", so it wouldn't have correct validation errors.

Yeah, at least I currently get them 😉

Lustmored · 2022-01-29T11:27:29Z

From a quick glimpse I can tell that this does not collide nor solve the problem addressed in my PR, which focuses on handling different kind of checkbox field types within forms 👍

weaverryan · 2022-01-31T20:28:28Z

src/LiveComponent/src/ComponentWithFormTrait.php

+        $form->submit($this->formValues);
+        // re-extract the "view" values in case the submitted data
+        // changed the underlying data or structure of the form
+        $this->formValues = $this->extractFormValues($this->getForm());


This is the key. It's super simple, it just took me a long time to understand what's going on. The flow is this:

A) A custom action is called
B) In the custom action, the user changes $this->formValues. For example, to add a new array to initialize a new collection type.
C) We re-extract the formValues from the form.

Normally, if the user sends formValues and we then use those to submit the form, the formValues that should be sent back to the frontend would be the same: that is STILL the "formValues" data. However, if your form changes as a result of submitted data (e.g. CollectionType which adds more fields if you add a new item to a collection type array), then we need to re-extract the formValues so that they match the new reality.

weaverryan · 2022-02-01T01:32:08Z

src/LiveComponent/src/ComponentWithFormTrait.php

-        }
-
-        return $this->formValues;
+        $this->formValues = $this->extractFormValues($this->getForm());
    }


Everything above this was just cleaning up the flow, but not significant

weaverryan · 2022-02-01T01:35:21Z

Other than a small TODO in the documentation, this is ready to go. This is much simpler now & mirrors what @norkunas was doing in his project, except now with the fix to formValues. It mentally took me awhile to get the clarity that this small solution was needed (I previously had a much bigger solution in this PR).

So if I'll have hundred live actions I'll need to write same number submit/reset statements? :)

So now the answer to that is "no": your actions modify formValues and nothing else.

src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php

kbond · 2022-02-01T14:47:13Z

src/LiveComponent/src/ComponentWithFormTrait.php

+     * If you override, call $this->initializeFormValues() manually
+     * and also $this->formView = $form if you are passing in a Form.
+     *
+     * TODO: Refactor to PostMount when available.


PostMount is now available.

yeeeea it is 😎

weaverryan · 2022-02-01T19:37:56Z

This is now ready. The form process is now much smoother, working with CollectionType is a wonderful experience.

norkunas · 2022-02-02T06:26:26Z

src/LiveComponent/src/Util/LiveFormUtility.php

+
+    public static function doesFormContainAnyErrors(FormView $formView): bool
+    {
+        if ($formView->vars['errors'] ?? null && \count($formView->vars['errors']) > 0) {


Maybe?:

Suggested change

if ($formView->vars['errors'] ?? null && \count($formView->vars['errors']) > 0) {

if (($formView->vars['errors'] ?? null) && \count($formView->vars['errors']) > 0) {

As EA hints about:

Not sure - but your suggestion is safer 👍

This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Twig] add `ExposeInTemplate` attribute | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Tickets | n/a | License | MIT #232 made all of a component's public properties available directly (without the `this.`) in it's template. This adds an `ExposeInTemplate` attribute to make non-public properties available in this way. Example: ```php use Symfony\UX\TwigComponent\Attribute\ExposeInTemplate; #[AsTwigComponent('with_exposed_variables')] final class WithExposedVariables { #[ExposeInTemplate] private string $prop1; // available as `{{ prop1 }}` #[ExposeInTemplate('customProp')] private string $prop2; // available as `{{ customProp }}` #[ExposeInTemplate(name: 'ico', getter: 'fetchIcon')] private string $icon = 'ico-warning'; // available as `{{ ico }}` in the template using `fetchIcon()` as the getter /** * Required to access $prop1 */ public function getProp1(): string { return $this->prop1; } /** * Required to access $prop1 */ public function getProp2(): string { return $this->prop2; } /** * Required to access $this->icon */ public function fetchIcon(): string { return $this->icon; } } ``` **TODO:** - [x] use to expose `form` in `ComponentWithFormTrait` (requires #254) Commits ------- e6c53fd [Twig] add `ExposeInTemplate` attribute

This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Twig] add `ExposeInTemplate` attribute | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Tickets | n/a | License | MIT #232 made all of a component's public properties available directly (without the `this.`) in it's template. This adds an `ExposeInTemplate` attribute to make non-public properties available in this way. Example: ```php use Symfony\UX\TwigComponent\Attribute\ExposeInTemplate; #[AsTwigComponent('with_exposed_variables')] final class WithExposedVariables { #[ExposeInTemplate] private string $prop1; // available as `{{ prop1 }}` #[ExposeInTemplate('customProp')] private string $prop2; // available as `{{ customProp }}` #[ExposeInTemplate(name: 'ico', getter: 'fetchIcon')] private string $icon = 'ico-warning'; // available as `{{ ico }}` in the template using `fetchIcon()` as the getter /** * Required to access $prop1 */ public function getProp1(): string { return $this->prop1; } /** * Required to access $prop1 */ public function getProp2(): string { return $this->prop2; } /** * Required to access $this->icon */ public function fetchIcon(): string { return $this->icon; } } ``` **TODO:** - [x] use to expose `form` in `ComponentWithFormTrait` (requires symfony/ux#254) Commits ------- e6c53fd [Twig] add `ExposeInTemplate` attribute

weaverryan force-pushed the allow-form-reset branch from 7658fa3 to b4d20b6 Compare January 31, 2022 20:24

weaverryan changed the title ~~Adding refreshForm() to ComponentWithFormTrait~~ [Live] Re-extracting form data after submit Jan 31, 2022

weaverryan commented Feb 1, 2022

View reviewed changes

weaverryan force-pushed the allow-form-reset branch from 0c6c713 to da9fc63 Compare February 1, 2022 01:50

kbond reviewed Feb 1, 2022

View reviewed changes

src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php Outdated Show resolved Hide resolved

kbond reviewed Feb 1, 2022

View reviewed changes

norkunas approved these changes Feb 1, 2022

View reviewed changes

kbond approved these changes Feb 1, 2022

View reviewed changes

kbond mentioned this pull request Feb 1, 2022

[Twig] add ExposeInTemplate attribute #259

Merged

1 task

norkunas reviewed Feb 2, 2022

View reviewed changes

[Live] Re-extracting form data after submit

c8b3a28

weaverryan force-pushed the allow-form-reset branch from e3a9104 to c8b3a28 Compare February 2, 2022 18:14

weaverryan merged commit 6715a03 into symfony:2.x Feb 2, 2022

weaverryan deleted the allow-form-reset branch February 2, 2022 18:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[Live] Re-extracting form data after submit #254

[Live] Re-extracting form data after submit #254

Uh oh!

weaverryan commented Jan 28, 2022 •

edited

Loading

Uh oh!

norkunas commented Jan 28, 2022

Uh oh!

weaverryan commented Jan 29, 2022 •

edited

Loading

Uh oh!

norkunas commented Jan 29, 2022

Uh oh!

Lustmored commented Jan 29, 2022

Uh oh!

weaverryan Jan 31, 2022

Uh oh!

weaverryan Feb 1, 2022

Uh oh!

weaverryan commented Feb 1, 2022

Uh oh!

Uh oh!

kbond Feb 1, 2022

Uh oh!

weaverryan Feb 1, 2022

Uh oh!

weaverryan commented Feb 1, 2022

Uh oh!

norkunas Feb 2, 2022

Uh oh!

weaverryan Feb 2, 2022

Uh oh!

Uh oh!

	if ($formView->vars['errors'] ?? null && \count($formView->vars['errors']) > 0) {
	if (($formView->vars['errors'] ?? null) && \count($formView->vars['errors']) > 0) {

Uh oh!

[Live] Re-extracting form data after submit #254

[Live] Re-extracting form data after submit #254

Uh oh!

Conversation

weaverryan commented Jan 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

norkunas commented Jan 28, 2022

Uh oh!

weaverryan commented Jan 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

norkunas commented Jan 29, 2022

Uh oh!

Lustmored commented Jan 29, 2022

Uh oh!

weaverryan Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

weaverryan Feb 1, 2022

Choose a reason for hiding this comment

Uh oh!

weaverryan commented Feb 1, 2022

Uh oh!

Uh oh!

kbond Feb 1, 2022

Choose a reason for hiding this comment

Uh oh!

weaverryan Feb 1, 2022

Choose a reason for hiding this comment

Uh oh!

weaverryan commented Feb 1, 2022

Uh oh!

norkunas Feb 2, 2022

Choose a reason for hiding this comment

Uh oh!

weaverryan Feb 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weaverryan commented Jan 28, 2022 •

edited

Loading

weaverryan commented Jan 29, 2022 •

edited

Loading