-
Notifications
You must be signed in to change notification settings - Fork 142
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
[Bug] Fix CSS for floatingFilter
in AgGrid
#388
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing! I would even add this as a fix to the fixed section. It definitely makes a difference to users. Thanks for taking care of this 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. 🚀
Description
floatingFilter
option buggy #383As already discussed, I think we need to evaluate on a case-by-case basis whether we fix the relevant CSS or whether the user needs to add the CSS themselves. In general, the default ag-grid theme should look fine without having to apply any custom CSS (e.g. the datepicker looks different than ours, but it looks fine still). The disadvantage of trying to cover every case is that our custom CSS grows and at some point gets difficult to maintain, because our overwrites might conflict. Long-term wise we should probably create a proper Figma design for the ag-grid, but that's currently not in the scope.
In this case, I agree we should fix it, as otherwise it's completely misaligned and it doesn't add much CSS.
Screenshot
Notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":