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

[DataGrid] Improve filters #635

Merged
merged 15 commits into from
Nov 25, 2020
Merged

[DataGrid] Improve filters #635

merged 15 commits into from
Nov 25, 2020

Conversation

dtassone
Copy link
Member

@dtassone dtassone commented Nov 24, 2020

Fix some of the ticks of #633

@dtassone dtassone changed the title ]Improve filters [DataGrid] Improve filters Nov 24, 2020
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

I'm not sure if I was supposed to start reviewing it, so I have left a few quick feedback :)

packages/grid/data-grid/src/DataGrid.tsx Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Nov 24, 2020
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

We are moving in the right direction :)

  • Regarding item 6. The close transition is now missing, but it's better than having a jump.
  • I have also noticed that the scroll is broken when opening the column menu. This is because of the autofocus on the input. There might be something wrong with the popper logic. A workaround is to delay the focus in an effect.

@oliviertassinari
Copy link
Member

A point we have started to discuss with @DanailH. I think that we should replace if (process.env.NODE_ENV === 'production') for the feature toggle, to a if (process.env.ENABLE_EXPERIMENTAL) flag. The issue with NODE_ENV is that it's 'production' in the Netlify preview. We can't easily review PRs, it forces us to check out each time. I'm working on it.

@dtassone
Copy link
Member Author

We are moving in the right direction :)

  • Regarding item 6. The close transition is now missing.
  • I have also noticed that the scroll is broken when opening the column menu

I'm not sure what you mean by the scroll.

can you try again the 6 and let me know if it's ok now :)

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 24, 2020

I'm not sure what you mean by the scroll.

Open the documentation, you will see, one negative point for storybook's isolation :p. I'm working on #635 (comment) so we can share a link. My env is broken so I can't share video yet.

@oliviertassinari
Copy link
Member

jump

@dtassone
Copy link
Member Author

this is caused by the autoFocus prop on the TextField, maybe a bug in core?

I refactored and fixed the issue

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 24, 2020

this is caused by the autoFocus prop on the TextField, maybe a bug in core?

autoFocus is a native attribute, actually discouraged for a11y, but that doesn't really matter here. Yes, it could be an issue with the Popper component.

@dtassone dtassone merged commit d523270 into mui:master Nov 25, 2020
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.

3 participants