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

KeyValue field updates state from the second time #12824

Open
finoghentov opened this issue May 16, 2024 · 0 comments
Open

KeyValue field updates state from the second time #12824

finoghentov opened this issue May 16, 2024 · 0 comments
Labels
Milestone

Comments

@finoghentov
Copy link

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

@zepfietje zepfietje added this to the v3 milestone Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

No branches or pull requests

2 participants