-
-
Notifications
You must be signed in to change notification settings - Fork 392
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
OBPIH-5909 Reset pagination on each filtered list table search #4337
OBPIH-5909 Reset pagination on each filtered list table search #4337
Conversation
disabled={!allowEmptySubmit && _.every(values, value => !value)} | ||
variant="primary" | ||
type="button" | ||
type="submit" |
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.
is there a reason for that change or is it just a cosmetic change?
Are we sure that handleSubmit
that is triggered will be triggered with the values
(values used in the updateFilterParams(values)
?
I mean if there:
<Form
onSubmit={updateFilterParams}
it evaluates to onSubmit={values => updateFilterParams(values)}
or some other variable hides under values
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.
this is a cosmetic change.
We are already using onSubmit={updateFilterParams}
when pressing enter on input search which works as expected.
The values
from onClick={() => updateFilterParams(values)}
are coming from <Form />
render props
<Form
onSubmit={updateFilterParams}
initialValues={{ ...defaultValues }}
render={({ values, handleSubmit, form }) => {
formRef.current = form;
countFilled(values);
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 know where they come from, but was just wondering, if <Form>
then onSubmit
uses the values
that are defined in the render
, but I assume they do.
Was just thinking whether something was not previously working due to that - it'd be good in the future to add a comment in the PR that "this is only a cosmetic change", not to cause confusions ;)
No description provided.