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

valuePrepareFunction + valueStoreFunction is a mess #138

Closed
nssoft-rlagrange opened this issue Sep 13, 2023 · 4 comments
Closed

valuePrepareFunction + valueStoreFunction is a mess #138

nssoft-rlagrange opened this issue Sep 13, 2023 · 4 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@nssoft-rlagrange
Copy link

nssoft-rlagrange commented Sep 13, 2023

Hello,

I'm migrating from ng2-smart-table but found a lot of issues.

Previously, the grid would display the result of valuePrepareFunction, but would keep the initial value for editing purpose.
Now, valuePrepareFunction erase the data with the result when edition start.

To fix this, you got valueStoreFunction to translate back the new value, but this method is only called on actually edited cell.
For the other unedited cells, the valuePrepareFunction erase the bound data, but are not reverted back.

Here is a simple task which became unpossible to fix, on a decimal value :

cost: {
        title: 'Cost',
        type: 'number',
        isFilterable: false,
        valuePrepareFunction: (value) => { return value == null ? 'N/A' : this.intl.format(value)}, // format to local (in french : 123 456,78)
        valueStoreFunction:(value) => this.parse_float(value,' ',','), // parse back to a float value (remove spaces and replace ',' with '.' before parseFloat to get the value)
      },
amount: {
        title: 'Amount
        type: 'number',
        valuePrepareFunction: (value) => { return value == null ? 'N/A' : this.intl.format(value)}, // same
        valueStoreFunction:(value) => this.parse_float(value,' ',','), // same
      },

When I change the first value, it's formatted then parsed, but the second value become a formatted string and is not reverted to a number. The formatted string will be refused by the server as it can't be parsed as a decimal.

I now have to implement the createConfirm and editConfirm to parse again all values...

In your position, I would probably revert the valuePrepareFunction committing the raw value when edition start, and present the raw value for editing purpose.
If needed, you can provide a valueEditFunction to prepare the value before edition.
valueStoreFunction was a good idea.

Another option would be to force the call to valueStoreFunction when editing end on unedited cells.

@nssoft-rlagrange
Copy link
Author

nssoft-rlagrange commented Sep 13, 2023

Another use case for this madness : I have a readonly column with date that I just want to show formatted.
If I format the date in the valuePrepareFunction, it's impossible to get the initial value back after editing.

@nssoft-rlagrange
Copy link
Author

Hi Guys, just to tell you that I have fixed this locally.
I'm not familiar with github source control, but will try to push a pull request shortly.

@uap-universe uap-universe added this to the 3.1.0 milestone Sep 29, 2023
@uap-universe uap-universe added the bug Something isn't working label Sep 29, 2023
@uap-universe uap-universe self-assigned this Sep 29, 2023
@uap-universe
Copy link
Collaborator

Thank you for the report. The valuePrepareFunction was always a mess and confused the types of the values that are edited (see the original bug description #115 ).

I will look into this, when I have some time. But with a first quick test on the demo page I could not reproduce the behavior you describe. But I will definitely take some more time to look into this deeper.

@uap-universe
Copy link
Collaborator

Hi @nssoft-rlagrange

I have figured out that the main issue is that the newValue property of a cell is still storing the "prepared" value from the valuePrepareFunction at several places, meaning that the bugfix of #115 was never entirely complete.

However, the fact that the valueStoreFunction is only called, when the value was actually edited is on purpose. Why would you need to translate the value back, when it was not changed? Just keep the original value.

That said, the valueStoreFunction and valuePrepareFunction should agree on a contract that basically states

x === valueStoreFunction(valuePrepareFunction(x))

I do not see your implementation of this.parse_float(), but in your case that means, the function would need to translate 'N/A' to null. Also I don't know what exactly you want to do with illegal input, like e.g. arbitrary strings.

The domain of the valueStoreFunction is in general larger than the image of the valuePrepareFunction, which is something you would always need to take care of, somehow.

Anyway, the recent commit should fix the issue with the valuePrepareFunction and will be included in release 3.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants