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

Row cellValue not updating when columnValue.valuePath is updated (Ember >= 3.13.0) #808

Closed
napafundi opened this issue Jan 21, 2020 · 1 comment · Fixed by #878
Closed

Comments

@napafundi
Copy link

When using new versions of Ember (>= 3.13.0) ember-table isn't updating row.cellValue correctly even though columnValue.valuePath has been changed. This occurs when setting a new array of columns to the table.head.columns property.

Here is a simple Ember project displaying this issue:
ember-table-issue

@billdami
Copy link

billdami commented Jan 27, 2020

I can also confirm that this issue exists still even with the just released Ember v3.16.0 and v3.17.0-beta.1. Its worth noting that the first version of ember that this issue started occurring, 3.13, was the Octane "preview release", and contained a large amount of changes to Computed Properties, introducing tracked properties, etc. So it seems likely that these large internal changes to the object model have something to do with this bug.

For anyone else that encounters this, a very crude workaround can be implemented by forcing ember-table to recognize a change to the rows objects immediately after changing columns, e.g. passing in a new temp array, and then changing it back to the original rows array in the next runloop:

@tracked isChangingColumns = false;

@action
changeColumns() {
     //do something that changes `this.columns`...
    
    this.isChangingColumns = true;
    next(this, () => this.isChangingColumns = false);
}
<T.body @rows={{if this.isChangingColumns (array) this.rows}} />

HeroicEric pushed a commit to HeroicEric/ember-table that referenced this issue Mar 9, 2021
Fixes Addepar#808
Fixes Addepar#820

The idea behind this is basically the same as Addepar#567.

The value of `cellValue` is computed by grabbing the `columnValue.valuePath`
and then grabbing the value of the corresponding key on `rowValue`.
Basically something like `rowValue.${columnValue.valuePath}`, which is
not possible using any of the built in computed macros.

This basically uses an observer to watch for changes to
`columnValue.valuePath`, and defines/redefines `cellValue` as a computed
alias for the property at that path on `rowValue`. This ensures the
`cellValue` is correct if `columnValue.valuePath` or the actual value at
that path on `rowValue` changes. Observers aren't recommended but this
was already using one.

Previously this was solved by creating a more generic `dynamicAlias`
computed macro in Addepar#567. To be
honest, I was having a little trouble wrapping my head around all that
was happening in there but I think the changes in this PR accomplish the
same idea.

I'm not totally sure what the issue was with the other implementation
but, since the `dynamicAlias` macro was only used in this one place, it
feels OK to have a more simple implementation that is hard coded
specifically for this case.
HeroicEric added a commit to HeroicEric/ember-table that referenced this issue Mar 9, 2021
Fixes Addepar#808
Fixes Addepar#820

The idea behind this is basically the same as Addepar#567.

The value of `cellValue` is computed by grabbing the `columnValue.valuePath`
and then grabbing the value of the corresponding key on `rowValue`.
Basically something like `rowValue.${columnValue.valuePath}`, which is
not possible using any of the built in computed macros.

This basically uses an observer to watch for changes to
`columnValue.valuePath`, and defines/redefines `cellValue` as a computed
alias for the property at that path on `rowValue`. This ensures the
`cellValue` is correct if `columnValue.valuePath` or the actual value at
that path on `rowValue` changes. Observers aren't recommended but this
was already using one.

Previously this was solved by creating a more generic `dynamicAlias`
computed macro in Addepar#567. To be
honest, I was having a little trouble wrapping my head around all that
was happening in there but I think the changes in this PR accomplish the
same idea.

I'm not totally sure what the issue was with the other implementation
but, since the `dynamicAlias` macro was only used in this one place, it
feels OK to have a more simple implementation that is hard coded
specifically for this case.
HeroicEric added a commit to HeroicEric/ember-table that referenced this issue Mar 9, 2021
Fixes Addepar#808
Fixes Addepar#820

The idea behind this is basically the same as Addepar#567.

The value of `cellValue` is computed by grabbing the `columnValue.valuePath`
and then grabbing the value of the corresponding key on `rowValue`.
Basically something like `rowValue.${columnValue.valuePath}`, which is
not possible using any of the built in computed macros.

This basically uses an observer to watch for changes to
`columnValue.valuePath`, and defines/redefines `cellValue` as a computed
alias for the property at that path on `rowValue`. This ensures the
`cellValue` is correct if `columnValue.valuePath` or the actual value at
that path on `rowValue` changes. Observers aren't recommended but this
was already using one.

Previously this was solved by creating a more generic `dynamicAlias`
computed macro in Addepar#567. To be
honest, I was having a little trouble wrapping my head around all that
was happening in there but I think the changes in this PR accomplish the
same idea.

I'm not totally sure what the issue was with the other implementation
but, since the `dynamicAlias` macro was only used in this one place, it
feels OK to have a more simple implementation that is hard coded
specifically for this case.
mixonic pushed a commit that referenced this issue Mar 17, 2021
Fixes #808
Fixes #820

The idea behind this is basically the same as #567.

The value of `cellValue` is computed by grabbing the `columnValue.valuePath`
and then grabbing the value of the corresponding key on `rowValue`.
Basically something like `rowValue.${columnValue.valuePath}`, which is
not possible using any of the built in computed macros.

This basically uses an observer to watch for changes to
`columnValue.valuePath`, and defines/redefines `cellValue` as a computed
alias for the property at that path on `rowValue`. This ensures the
`cellValue` is correct if `columnValue.valuePath` or the actual value at
that path on `rowValue` changes. Observers aren't recommended but this
was already using one.

Previously this was solved by creating a more generic `dynamicAlias`
computed macro in #567. To be
honest, I was having a little trouble wrapping my head around all that
was happening in there but I think the changes in this PR accomplish the
same idea.

I'm not totally sure what the issue was with the other implementation
but, since the `dynamicAlias` macro was only used in this one place, it
feels OK to have a more simple implementation that is hard coded
specifically for this case.

Co-authored-by: Eric Kelly <HeroicEric@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants