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

Disable default aggrid column fit #1419

Merged
merged 2 commits into from
Aug 16, 2023

Conversation

qingant
Copy link
Contributor

@qingant qingant commented Aug 14, 2023

This is not desirable if you have a table with a big amount of columns. If you have 30+ columns, the column will be wrap and no header displayed.

We could control it by props but it might just work as expected in most of cases by simply disabling it.

This is not desirable if you have a table with a big amount of columns.
Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request, @qingant!
The auto-sizing of grid columns has caused quite some confusion in the past (e.g. #1121).
I still think for small grids it is a rather nice default behavior. And just to avoid breaking changes, I would keep it for now. But I took the opportunity to make it configurable. What do you think?

@falkoschindler falkoschindler added this to the 1.3.10 milestone Aug 16, 2023
@falkoschindler falkoschindler added the enhancement New feature or request label Aug 16, 2023
@qingant
Copy link
Contributor Author

qingant commented Aug 16, 2023

Thanks for the pull request, @qingant! The auto-sizing of grid columns has caused quite some confusion in the past (e.g. #1121). I still think for small grids it is a rather nice default behavior. And just to avoid breaking changes, I would keep it for now. But I took the opportunity to make it configurable. What do you think?

Sure, thanks for the context. I will working on making it configurable.

@falkoschindler
Copy link
Contributor

@qingant Oh, I already added the parameter auto_size_columns: bool = True to make it configurable. I just wanted to hear your opinion, whether you agree with this solution.

@qingant
Copy link
Contributor Author

qingant commented Aug 16, 2023

@qingant Oh, I already added the parameter auto_size_columns: bool = True to make it configurable. I just wanted to hear your opinion, whether you agree with this solution.

Thanks ! didn't realize that when I replied. agreed this is a good solution which gives the user the option to enable/disable it.

BTW: I noticed that this has been approved, should I merge it or it's better to wait someone else to merge ? asking this question to make sure I will do it right in the future when I want to contribute to this repo.

@falkoschindler falkoschindler merged commit 920bab3 into zauberzeug:main Aug 16, 2023
1 check passed
@falkoschindler
Copy link
Contributor

No worries, @qingant. I just merged it. (I think you don't have the permission to do so.)
Thanks again for your contribution! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants