-
Notifications
You must be signed in to change notification settings - Fork 204
feat: implement style api for text filter component #4050
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4050 +/- ##
==========================================
+ Coverage 97.00% 97.12% +0.11%
==========================================
Files 862 863 +1
Lines 25290 25292 +2
Branches 9123 9120 -3
==========================================
+ Hits 24532 24564 +32
+ Misses 752 681 -71
- Partials 6 47 +41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cabdfba to
67fe2fd
Compare
| [customCssProps.stylePlaceholderFontStyle]: style?.placeholder?.fontStyle, | ||
| }; | ||
|
|
||
| return properties; |
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.
I see slightly different implementations in styles.tsx files. E.g. in prompt input we filter out the undefined values, why aren't we doing the same here?
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.
Yea because of this comment: #4025 (comment)
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.
I see, then we should remove that logic from prompt input as well
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.
Yes, I'll open a subsequent PR for that if that's ok?
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.
Yes, also in the PR linked I don't see matchSnapshot being used but it is used here. Why are there different implementations? Let's have consistency across these similar implementations
67fe2fd to
8606da3
Compare
Description
Extends the text filter component with a style API for customization, allowing developers to override default component styles.
Related links, issue #, if available: n/a
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.