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

🐾 Update to PF5 - part II - onChange event handler #1269

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

sgratch
Copy link
Collaborator

@sgratch sgratch commented Jul 15, 2024

Reference: #1098

For avoiding uncaught PF 4 -> PF 5 migration errors, this PR named and typed (i.e. (add type to function signature) all onChange callback appearances which use parameters.

@sgratch sgratch requested a review from yaacov July 15, 2024 12:00
@@ -23,12 +23,19 @@ export const SearchInputProvider: React.FunctionComponent<SearchInputProviderPro
filterDispatch({ type: 'SET_NAME_FILTER', payload: value });
};

const onChange: (event: React.FormEvent<HTMLInputElement>, value: string) => void = (
event,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
event,
_event,

do you we use the event value ?

event: React.FormEvent<HTMLInputElement>,
value: string,
date?: Date,
) => void = (event, value) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
) => void = (event, value) => {
) => void = (_event, value) => {

see other places we get a value we don't use

minute?: number,
seconds?: number,
isValid?: boolean,
) => void = (event, time, hour, minute) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
) => void = (event, time, hour, minute) => {
) => void = (_event, _time, hour, minute) => {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A good catch. Apparently the linter is not shouting on these cases for some reason.
Anyway, I fixed all found places, including ones that I didn't add as part of this PR.

) => {
dispatch({ type: 'SET_USERNAME', payload: value });
};
const onChangePassword: (value: string, event: React.FormEvent<HTMLInputElement>) => void = (
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const onChangePassword: (value: string, event: React.FormEvent<HTMLInputElement>) => void = (
const onChangePassword: (value: string, event: React.FormEvent<HTMLInputElement>) => void = (

adding empty line between the methods

@@ -144,6 +144,17 @@ export const PlansCreateForm = ({

const networkMessages = buildNetworkMessages(t);
const storageMessages = buildStorageMessages(t);
const onChangePlan: (value: string, event: React.FormEvent<HTMLInputElement>) => void = (
Copy link
Member

Choose a reason for hiding this comment

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

empty line before the methods, here and in L152

@sgratch sgratch force-pushed the onChange_event_handler_for_pf5 branch from 1d0c35d to 5930a24 Compare July 16, 2024 14:20
Reference: kubev2v#1098

For avoiding uncaught PF 4 -> PF 5 migration errors, this PR named and typed (i.e. (add type to function signature) all onChange callback appearances which use parameters.

Signed-off-by: Sharon Gratch <[email protected]>
@sgratch sgratch force-pushed the onChange_event_handler_for_pf5 branch from 5930a24 to 29cb8ae Compare July 16, 2024 14:43
@sgratch sgratch requested a review from yaacov July 16, 2024 14:52
@yaacov yaacov merged commit ae6d381 into kubev2v:main Jul 16, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants