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

[data grid] onFilterModelChange fires with stale field value #10711

Closed
wants to merge 11 commits into from
Closed

[data grid] onFilterModelChange fires with stale field value #10711

wants to merge 11 commits into from

Conversation

gitstart
Copy link
Contributor

Description

Upon investigation, we discovered that this issue occurs when a user wants to filter a value in a String column and then tries to change the String Column to a Number column. When this happens, the program tries to parse the string value the user initially inputted in the Value Text field to a number value, because it's unable to do that, it clears out the value from the Text Field but the value still remains in the items object in the program.

The solution to this would be to reset the item value property when the user switches from one column to another.

Loom Video

closes #10489


This code was written and reviewed by GitStart Community. Growing great engineers, one PR at a time.

Co-authored-by: jasmyneokudo <38866525+jasmyneokudo@users.noreply.github.com>
@mui-bot
Copy link

mui-bot commented Oct 18, 2023

Deploy preview: https://deploy-preview-10711--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 152803a

gitstart and others added 10 commits October 19, 2023 06:52
Co-authored-by: jasmyneokudo <38866525+jasmyneokudo@users.noreply.github.com>
Co-authored-by: jasmyneokudo <38866525+jasmyneokudo@users.noreply.github.com>
Co-authored-by: jasmyneokudo <38866525+jasmyneokudo@users.noreply.github.com>
Co-authored-by: jasmyneokudo <38866525+jasmyneokudo@users.noreply.github.com>
Co-authored-by: jasmyneokudo <38866525+jasmyneokudo@users.noreply.github.com>
@gitstart gitstart marked this pull request as ready for review October 19, 2023 19:35
@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Oct 20, 2023
@alexfauquette alexfauquette changed the title MUIX-10489 - [data grid] onFilterModelChange fires with stale field value [data grid] onFilterModelChange fires with stale field value Oct 27, 2023
@@ -183,11 +183,11 @@ describe('<DataGrid /> - Filter panel', () => {

setColumnValue('slogan');

expect(getColumnValues(0)).to.deep.equal([]);
expect(getColumnValues(0)).to.deep.equal(['Nike', 'Adidas', 'Puma']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into it!
While the fix is quite simple, it changed the filtering behavior in some valid use cases - this is why you had to update a few unit tests for them to pass.

Instead of always resetting the filter value, did you consider other options?
For example, resetting the filter value only if the type of the filtered column changes.
What do you think?

@michelengelen michelengelen changed the base branch from master to next November 6, 2023 14:20
@GitStartHQ GitStartHQ closed this by deleting the head repository Nov 7, 2023
@burakkgunduzz
Copy link
Contributor

Is there any updates here? Why you closed the PR? @gitstart This fix is important for me and was looking forward to see it.

@gitstart
Copy link
Contributor Author

Is there any updates here? Why you closed the PR? @gitstart This fix is important for me and was looking forward to see it.

@burakkgunduzz the PR got closed due to some internal cleanups on our end.
However, a new PR has been created on #11000 and the feedback implementation will be pushed there shortly.
Apologies for the inconvenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] onFilterModelChange fires with stale field value
6 participants