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

[LiveComponent] Updated values not submitted to the server properly #1557

Closed
lemorragia opened this issue Feb 28, 2024 · 7 comments · May be fixed by #1572
Closed

[LiveComponent] Updated values not submitted to the server properly #1557

lemorragia opened this issue Feb 28, 2024 · 7 comments · May be fixed by #1572

Comments

@lemorragia
Copy link

lemorragia commented Feb 28, 2024

I've got an update problem, with 2.14.2 version of twig+live components. The use case is: a Sale Order has many items. I'm using a Live Component + Form + LiveCollectionType.
Every item has a "product", a "price", a "quantity" and an "untaxedTotal". Ideally when you select a product, the price is updated with the product salePrice, and when you change quantity or price on the SaleOrderItem, the untaxedTotal is also updated.

OrderComponent

#[AsLiveComponent]
final class OrderComponent extends AbstractController
{
    use ComponentWithFormTrait;
    use DefaultActionTrait;
    use LiveCollectionTrait;

    #[LiveProp(writable: true)]
    public ?SaleOrderInput $initialFormData = null;

    protected function instantiateForm(): FormInterface
    {
        return $this->createForm(OrderType::class, $this->initialFormData);
    }

    #[LiveAction]
    public function updateUnitPrice(): void
    {
        foreach ($this->formValues['items'] as &$item) {
            $item['unitPrice'] = rand(0, 40);
            if (empty($item['quantity'])) {
                $item['quantity'] = 1;
            }
            $this->setUntaxedTotal($item);
        }
    }

    #[LiveAction]
    public function updateUntaxedTotal(): void
    {
        foreach ($this->formValues['items'] as &$item) {
            $this->setUntaxedTotal($item);
        }
    }

    private function setUntaxedTotal(array &$item): void
    {
        $item['untaxedTotal'] = $item['unitPrice'] * $item['quantity'] - $item['discount'];
    }
}

and the relative form (simplified):

class OrderType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add('items', LiveCollectionType::class, [
                'entry_type' => OrderItemType::class,
                'entry_options' => ['label' => false],
                'label' => false,
                'allow_add' => true,
                'allow_delete' => true,
                'by_reference' => false,
            ])
        ;
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'data_class' => SaleOrderInput::class
        ]);
    }
}

also the OrderItemType:

class OrderItemType extends AbstractType
{
    /** @param array<string, mixed> $options */
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder = new DynamicFormBuilder($builder);

        $builder
            ->add('product', EntityType::class, [
                'class' => Product::class,
                'required' => false,
                'label' => 'wd.field.product',
                'autocomplete' => true,
                'attr' => [
                    'placeholder' => 'wd.field.product',
                    'data-action' => 'live#action',
                    'data-action-name' => 'updateUnitPrice',
                ],
            ])
            ->add('quantity', NumberType::class, [
                'label' => 'wd.field.quantity',
                'scale' => 1,
                'required' => true,
                'html5' => true,
                'attr' => [
                    'data-action' => 'live#action',
                    'data-action-name' => 'updateUntaxedTotal',
                ],
            ])
            ->add('unitPrice', NumberType::class, [
                'label' => 'wd.field.unitPrice',
                'scale' => 1,
                'required' => true,
                'html5' => true,
                'attr' => [
                    'data-action' => 'live#action',
                    'data-action-name' => 'updateUntaxedTotal',
                ],
            ])
            ->add('untaxedTotal', NumberType::class, [
                'label' => 'wd.field.total',
                'attr' => [
                    'readonly' => true,
                ],
            ])
        ;

        [..]
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'data_class' => SaleOrderItemInput::class,
        ]);
    }
}

When i select a product all the form fields are updated correctly. The problem is that when i try to change manually quantity or unitPrice, the untaxedTotal stays always "one step back".
Let's say that unitPrice is 1 and quantity is 1, if i click to increase the NumberType or if i manually edit the number to "2" a UX ajax call is made, with an empty "updated" object. On the second change (to "3") another call is made with an updated object containing "2" as the new value. This is true with an "html5"-> false setting, or even with a TextType as form field.

Something funny could be going on between HTML/DOM and UX, because on the first change the value attribute in the html "rightfully" remains "one step behind" (so on the first change, the value remains "1" in the HTML). So maybe i'm wrong using a LiveAction to update the form data for this use case, or some additional javascript is required to force the value of the HTML to match the one displayed, or UX is not catching the proper updated value correctly. In the most basic test "add a collection element"->"select a product"->"increase the unitPrice by 1" it seems to me that UX should send the updated value inside the "updated" object of the ajax call. Any ideas about why this is happening?

EDIT: i actually built a simple prototype, and the behaviour is the same. On further testing, if i keep the focus on the fields and as long as i edit them by typing, all the ajax calls are made, but the value is never changed, even after N edits. If i edit the field, lose the focus, and edit it again, the values start to change (but always "one step back"). I also tried a different syntax to bind to different events (eg. "keyup->live#action") but with no change, i doubt that the problem is event binding, because when i edit the field the ajax calls are always launched correctly, even though with "not updated data". This is also true with twig/live components 2.15.0

@weaverryan
Copy link
Member

Hey!

Can you share your simple prototype on GitHub? That'd be super helpful :)

@lemorragia
Copy link
Author

Sure, you can have a look at it here . I tried to simplify it as much as possible. To reproduce: follow the readme instructions to have it up and running. There's only a single page now (the "new"). You can add an item to the collection/order items, select a product, and try to change quantity and/or unitPrice.

You'll see that UX ajax calls are made when one expects it (every time you type or using the NumberType arrows), but on the first input/change the "updated" object is empty...also if you type in the field, as long as you mantain the focus on it, ajax calls are made but no changes will happen no matter how many times you type (i guess this is related to the input/change events of the input itself, the second one fired only using the input arrows/losing focus and the first when you type, but UX is bound to both events).

I did a little bit more of digging inside "live_controller.js", and it seems that getValueFromElement->inputValue functions return the correct value of the input as is shown while you're editing it. The value also is properly get as "element.value". It almost seems as if the data for the ajax call is collected(or the ajax call itself is made) before calling the getValueFromElement, but i'm starting to hit my limits on javascript knowledge here :-/.

I also tried some other versions of ux (2.14.*) with no differences, and the behaviour is not influenced by other ux libraries (autocomplete/tom-select, etc..) that i use separately as far as i can tell. Trying to bind to different events in ux (like keyup, fe) doesn't make a difference either.

@smnandre
Copy link
Collaborator

smnandre commented Mar 1, 2024

Two things: could the live action on your input be triggered before the model change ?

But mainly i guess it's because the form data is not submitted when you access it in your Live Action: https://symfony.com/bundles/ux-live-component/current/index.html#using-form-data-in-a-liveaction

@lemorragia
Copy link
Author

@smnandre i'm not trying to submit data or change the model yet, i'm just trying to dynamically update the form as explained in https://symfony.com/bundles/ux-live-component/current/index.html#dynamically-updating-the-form-in-a-liveaction. I guess that submitting the form and saving the object will send the correct value to the server, but that's not what i'm trying to do.

My guess right now is that UX listens to both "input" and "change" events on the html input and there's something strange happening there collecting the value of the field

@weaverryan
Copy link
Member

weaverryan commented Mar 1, 2024

UPDATE: Actually, I think #1572 is not needed at all. I think this is just a misconfiguration (and an easy one to do). We already handle this case, but your action and model update need to happen on the same event.


Thanks for the excellent reproducer 🥳. Fixed in #1572. This functionality actually existed ages ago, but was lost at some point. There is one change you need to make to your code - and it's explained on that PR:

-'data-action' => 'live#action',
+'data-action' => 'change->live#action',

Otherwise, you're triggering the action on the input event (which happens first) but then the model isn't updated until "later" (usually around 100-150 ms on my computer) in the change event). You can kinda see the problem too: if you type into the quantity box, a bunch of unnecessary Ajax calls to updateTotal are made (unnecessary because the model isn't updating at all, until the field loses focus).

Btw, this use-case should be supported, hence the PR. But in general, I think a better approach would be either:

A) Instead of, on change, calculating things like the total, calculate them at runtime in your template - e.g. call a getTotal() which runs the calculations. However, I see that you're trying to keep the values set onto the form.

B) Use an updated callback. Basically, there is already a mechanism to run some code when a specific model is updated. Though, hmm, I see a problem here too! The formValues is the LiveProp you want to be notified on for an update (or you can listen to specific key... but that's not the point). As you don't "own" this property (it's in the trait), you can't add the onUpdated to it. 🤔. We should think of a solution for this :)

@smnandre
Copy link
Collaborator

smnandre commented Mar 1, 2024

And guys, that’s why Ryan is the boss.

👏

@lemorragia
Copy link
Author

Thank you @weaverryan for the very interesting and compelling answer! Yes, i tried your change and it works, whenever the element is changed. As far as i understand it, and you're explaining it, UX collects changes on the element only on the "change" event, and there's no way to bind that kind behaviour for example on a different event (eg. keyup)...which i admit could be kind of a pointless/edge case, but i'm just trying to understand.

I'm trying "a lot of UX" as a proof of concept, so having a proper custom template with a disabled field/something else with a "getTotal" method is one of the things i'm gonna try. The use case i'm trying is much more complicated than this one (including DTOs, dependent fields, autocomplete, etc), because i'm trying to see what UX can do (and kind-of see if i can kind-of abandon js ;-) )

The updated callback case you mention is also very interesting and i can see why it can be tricky. In a sense, given that you're trying to "listen and manipulate only the view" here, the "launch some callback when updated" it's kind-of the case we're exploring: attribute on the html -> call a LiveAction -> manipulate the formValues.

Maybe it's only the "$this->formValues['...']" array syntax that looks strange for this particular case and could be a little nicer with some specific method and/or expanded docs on "manipulating the form/view only" before actually submitting the data. Gonna close the issue in the meantime, and thanks again for the support!

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 a pull request may close this issue.

3 participants