-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
feat: simpler integration filters #4766
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
2a79478
to
32f2dad
Compare
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.
Happy to give it an approval tomorrow after looking at the comment. I'm not approving right now because of the auto-merge
required, | ||
}) => { | ||
const renderInput = (params: AutocompleteRenderInputParams) => ( | ||
<TextField | ||
{...params} | ||
error={Boolean(error)} | ||
helperText={error} | ||
helperText={error || note} |
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.
If this is error or note, maybe we should have a single text and another property of type style: 'error' || 'note'
?
Personally, I don't like positional parameters having different meanings, and they seem to be mutually exclusive (i.e. when error is undefined you can have a note, when there's an error, it takes precedence over the note, so they can't be rendered together...
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.
So they're different texts, but the error is "dynamic" in a way. Something can trigger a validation that will set the error state and will display the error.
https://linear.app/unleash/issue/2-1407/remove-the-all-checkboxes-from-project-and-environment-filters
Simplifies integration event filters by removing the "ALL" checkboxes from these components. Whether you opted to check the "ALL" checkbox, or not to filter at all, the result is the same - The selected options would act as a filter.
Includes some refactoring and clean up.