filament icon indicating copy to clipboard operation
filament copied to clipboard

KeyValue field updates state from the second time

Open finoghentov opened this issue 1 year ago • 8 comments

Package

filament/filament

Package Version

3.2

Laravel Version

10.10

Livewire Version

3.4

PHP Version

8.1

Problem description

I have a simple resource editing form.

KeyValue::make('options'),
Select::make('type')
  ->options(array_column(BoostType::cases(), 'name'))
  ->afterStateUpdated(function (Component $component, Set $set) {
        $set('options', ['test' => rand(0, 1000)]);
  })
  ->nullable(false)
  ->live()

Key value update state only from the second time

The problem in this script

Initializing the component with NON empty value triggers updateState method which switches shouldUpdateRows flag to false

https://github.com/filamentphp/filament/blob/3.x/packages/forms/resources/js/components/key-value.js#L15

if (this.rows.length <= 0) {
    this.rows.push({ key: '', value: '' })
} else {
    this.updateState()
}

https://github.com/filamentphp/filament/blob/3.x/packages/forms/resources/js/components/key-value.js#L105

this.shouldUpdateRows = false

While first updating field state we trigger next code which switch flag to true https://github.com/filamentphp/filament/blob/3.x/packages/forms/resources/js/components/key-value.js#L72

if (!this.shouldUpdateRows) {
    this.shouldUpdateRows = true
    return
}

I've found that this was required to fix this bug but we need to find a better decision. Maybe something like this:

updateState: function () {
    let state = {}

    let keys = [];

    for (let i in this.rows) {
        if (
            keys.includes(this.rows[i].key) ||
            this.rows[i].key === '' ||
            this.rows[i].key === null
        ) {
            return;
        }

        keys.push(this.rows[i].key);
        state[this.rows[i].key] = this.rows[i].value
    }

    this.state = state
},

Remove shouldUpdate flag and do not update the state if we've found empty or duplicated keys. But maybe we should make a warning to make users understand that with duplicated keys state won't be updated

Expected behavior

I expect that changing the Select value will update the KeyValue state, but it works only If I change Select 2 times. It works only if you are updating the resource with some filled value in KeyValue.

Steps to reproduce

  • Create resource
  • Paste example code in resource editing form
  • Create resource entity and put some json to entity in database
  • Update created entity and try to change select value

Reproduction repository

https://github.com/finoghentov/filament-key-value-bug

Relevant log output

No response

finoghentov avatar May 16 '24 12:05 finoghentov

Do you still have the problem? I just tried the reproduction repository and it seemed to work.

https://github.com/filamentphp/filament/assets/10696975/d82e4774-0bcc-476c-8d92-87c3728a2586

alexmanase avatar Jun 04 '24 18:06 alexmanase

Do you still have the problem? I just tried the reproduction repository and it seemed to work.

Screen.Recording.2024-06-04.at.21.37.00.mov

To reproduce this bug you should do an update of resource.

finoghentov avatar Jun 06 '24 11:06 finoghentov

I added ->live() to the KeyValue input and it worked for me 🙂

alexmanase avatar Jun 06 '24 12:06 alexmanase

I added ->live() to the KeyValue input and it worked for me 🙂

image image image

It works only from the second change. Please make sure, that you check it in editing form

finoghentov avatar Jun 11 '24 14:06 finoghentov

Yes, I tried from the editing form.

https://github.com/filamentphp/filament/assets/10696975/bc5f27ce-ca4f-4510-97e2-b8131b63c23d

alexmanase avatar Jun 11 '24 16:06 alexmanase

Facing a similar issue with the Select components' afterStateUpdated, it only triggers on the 2nd select not the 1st. Any fixes yet? image

firascodes avatar Oct 22 '24 07:10 firascodes

This PR https://github.com/filamentphp/filament/issues/12824 is maybe related with this issue.

borjajimnz avatar Jan 16 '25 22:01 borjajimnz

Hey @polar-sh[bot]! We're sorry to hear that you've hit this issue. 💛

However, it looks like you forgot to fill in the reproduction repository URL. Can you edit your original post and then we'll look at your issue?

We need a public GitHub repository which contains a Laravel app with the minimal amount of Filament code to reproduce the problem. Please do not link to your actual project, what we need instead is a minimal reproduction in a fresh project without any unnecessary code. This means it doesn't matter if your real project is private / confidential, since we want a link to a separate, isolated reproduction. That would allow us to download it and review your bug much easier, so it can be fixed quicker. Please make sure to include a database seeder with everything we need to set the app up quickly.

github-actions[bot] avatar Mar 20 '25 17:03 github-actions[bot]

I’m working on a solution for both issues:

Summary of proposed changes

  • updateRows:
I’m using the stateRows array to preserve any changes that the user is making. It’s a copy of the state in array form. The extraRows are rows that the user has entered that are duplicates or empty and therefore not updated to the state yet. Rows are only updated when they don't match the state.
  • updateState:
Sync the rows but only the ones that don’t have an issue. (@finoghentov solution)
  • Validation:
However if a user enters a duplicate key/value, the responsibility of checking this falls on the backend validation. This will not generate an error if a user enters a duplicate key and saves the form, it will silently skip the attempt. If we want front end validation I propose a new method to handle this.

The issue with @finoghentov solution is that if you don’t track user changes the UI updates without the item that is currently being edited if it's empty or a duplicate.

https://github.com/filamentphp/filament/pull/16290

alex-elivate avatar May 19 '25 02:05 alex-elivate

Fixed in v4 by #16390

danharrin avatar Jun 07 '25 19:06 danharrin