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] Add prop to override search input props in GridColumnsManagement #15347

Merged
merged 19 commits into from
Nov 18, 2024

Conversation

k-rajat19
Copy link
Contributor

@k-rajat19 k-rajat19 commented Nov 8, 2024

@mui-bot
Copy link

mui-bot commented Nov 8, 2024

Localization writing tips ✍️

Seems you are updating localization 🌍 files.

Thank you for contributing to the localization! 🎉 To make your PR perfect, here is a list of elements to check: ✔️

  • Verify if the PR title respects the release format. Here are two examples (depending if you update or add a locale file)

    [l10n] Improve Swedish (sv-SE) locale
    [l10n] Add Danish (da-DK) locale

  • Update the documentation of supported locales by running pnpm l10n
  • Clean files with pnpm prettier.

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

Generated by 🚫 dangerJS against 369e2a2

@k-rajat19 k-rajat19 marked this pull request as ready for review November 8, 2024 16:54
}}
autoComplete="off"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

visibility: 'hidden',
},
]}
tabIndex={-1}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to align with #14587

@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Nov 11, 2024
<rootProps.slots.quickFilterClearIcon fontSize="small" />
</rootProps.slots.baseIconButton>
),
sx: {
Copy link
Member

Choose a reason for hiding this comment

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

I think we try to avoid the sx prop where possible in the grid - could we use the styled util for these styles instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have shifted those styles to its parent which uses styled, it selects the input with type="search" and applies the styles to it.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better in it's own styled for the input itself, it's clearer that the styles belong to that element.

Copy link
Contributor Author

@k-rajat19 k-rajat19 Nov 13, 2024

Choose a reason for hiding this comment

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

I've added it,
also tried using styled only for input in inputComponent prop inside InputProps but it didn't seem to work well.
followed this section of docs https://mui.com/material-ui/react-text-field/#using-the-styled-api
do we also need to add a class for style overrides?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think we should have a class on it for overrides.

Copy link
Member

Choose a reason for hiding this comment

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

Seems ok for me - I have pushed the changes it generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I have resolved the merge conflicts
would you like to give it a final look before merging

Copy link
Member

Choose a reason for hiding this comment

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

Looks like there is a slight size difference in the panel now 🤔

Shouldn't be a big problem, but any idea why? https://app.argos-ci.com/mui/mui-x/builds/26671/119787928

Copy link
Contributor Author

@k-rajat19 k-rajat19 Nov 18, 2024

Choose a reason for hiding this comment

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

this is due to the addition of endAdornment property, mui text field internally adds a different class which brings the right padding. I've set equal padding to the left and right both 12px.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-11-18 172621

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 14, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 14, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 15, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 18, 2024
Copy link
Member

@KenanYusuf KenanYusuf left a comment

Choose a reason for hiding this comment

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

Thanks a lot for working on this 🎉

@KenanYusuf KenanYusuf enabled auto-merge (squash) November 18, 2024 15:35
@KenanYusuf KenanYusuf merged commit 09a70a7 into mui:master Nov 18, 2024
18 checks passed
@KenanYusuf KenanYusuf added needs cherry-pick The PR should be cherry-picked to master after merge v7.x labels Nov 18, 2024
KenanYusuf added a commit to KenanYusuf/mui-x that referenced this pull request Nov 19, 2024
…agement` (mui#15347)

Co-authored-by: Kenan Yusuf <kenan.m.yusuf@gmail.com>
@k-rajat19 k-rajat19 deleted the issue-15055 branch November 19, 2024 10:28
LukasTy pushed a commit to LukasTy/mui-x that referenced this pull request Dec 19, 2024
…agement` (mui#15347)

Co-authored-by: Kenan Yusuf <kenan.m.yusuf@gmail.com>
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! needs cherry-pick The PR should be cherry-picked to master after merge v7.x
Projects
None yet
4 participants