ariaieboy / filament-currency

Enhanced Currency Related stuff for Filament
MIT License
42 stars 8 forks source link

[Bug]: Typing live jumps in afterStateUpdated #32

Closed nielsvh2103 closed 4 months ago

nielsvh2103 commented 4 months ago

What happened?

When typing, its updated automatically and changes values on afterStateUpdated

E.g. your last commit to 1.7.5 breaks the fact of automatic typing update (live)

See video:

https://github.com/ariaieboy/filament-currency/assets/60341437/86a22a72-5ecb-473c-9f17-29ddc9b2e427

How to reproduce the bug

Have an afterStateUpdated with a $set.

I can't provide you an example, but what we are doing is we are typing a price in a repeater and then another variable is being updated to set a total price of all repeater items.

               $totalPrice = collect($get('items'))
                                    ->map(function($item) {
                                        if($item && (Arr::exists($item, 'quantity') && Arr::exists($item, 'price'))) {
                                            return ((int)$item['quantity']) * ((float)$item['price']);
                                        }

                                        return null;
                                    })
                                    ->sum();

                                $set('totalPrice', $totalPrice);

This is what we are doing

This bug is created by the following;

 if(this.masked?.replaceAll('$thousandSeparator','').replaceAll('$decimalSeparator','.') != this.input){
                    this.masked = this.input?.toString().replaceAll('.','$decimalSeparator');
                }

If i revert it back to this, its fixed.

                this.masked = this.input?.toString().replaceAll('.','$decimalSeparator');
                \$el.dispatchEvent(new Event('input'));

Screenshot 2024-05-31 at 14 54 45

Here by logging you can see what happens.

console.log(this.masked?.replaceAll('$thousandSeparator','').replaceAll('$decimalSeparator','.') != this.input);

while typing is the issue here, its going true false true false because of nextTicking. Therefore this fix for 1.7.5 won't do.

Ill make a PR but granted it might break issue #31 again.

Package Version

1.7.5

PHP Version

8.3.7

Laravel Version

11.9

Which operating systems does with happen with?

macOS

Notes

No response

ariaieboy commented 4 months ago

This issue is not specific to our package the filament live update has this problem too.

just use this snippet:

                Forms\Components\TextInput::make('test2'),
                Forms\Components\TextInput::make('test1')->live()
                    ->afterStateUpdated(function (Forms\Get $get, Forms\Set $set,$state) {
                    sleep(1);
                    $set('test2',$get('test1'));
                }),

the sleep method is there to simulate a poor connection. Since with the live option, each change goes back to the server if you type too fast it's going to replace your input with the last data that it's received from you in the request.

you should set a debounce to prevent this but right now we don't support debounce nor the filament on the fields that use Alpinejs.

I am going to work on adding debounce and blur support tomorrow.

nielsvh2103 commented 4 months ago

Alright thanks

Though in the PR, re adding the event dispatch fixed the issue for me. Did not experience it until i went from 1.7.3 to 1.7.5

Since with the live option, each change goes back to the server if you type too fast it's going to replace your input with the last data that it's received from you in the request.

This does not happen on regular inputs because updateMasked is actually replacing client side and can't keep up, therefore its probably broken with alpine + livewire .live combined (as you state), just having livewire .live on an input without formatting or changing it client side works fine.

Though re adding \$el.dispatchEvent(new Event('input')); fixed the issue for me (in 1.7.5)

For example going back to 1.7.3 it works perfectly for us, works fine on using $set aswell. Tried inside and outside a repeater, so not sure why it broke in 1.7.3 in #31

Either way adding onBlur will give the option to not update the mask everytime when typing and works for us.

Thanks for fast responses! Really need this mask as what filament provides using RawJs::make('$money($input)') only works client side and not actually sets the data properly to save :')

ariaieboy commented 4 months ago

you can set a debounce of 150 to prevent this on v1.8.0

nielsvh2103 commented 3 months ago

It works properly now when we use onBlur, thanks alot