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

Added required field check in wizard flow #1039

Merged

Conversation

GowthamShanmugam
Copy link
Contributor

@GowthamShanmugam
Copy link
Contributor Author

GowthamShanmugam commented Sep 11, 2023

Screencast.from.2023-09-12.15-46-00.webm

@GowthamShanmugam GowthamShanmugam changed the title Added required field check in wizard flow - WIP Added required field check in wizard flow Sep 11, 2023
@GowthamShanmugam GowthamShanmugam force-pushed the bz_2236436 branch 2 times, most recently from 292b117 to 8973ca2 Compare September 12, 2023 05:41
setStepIdReached(stepIdReached < id ? id : stepIdReached);
}
};
const [isValidationEnabled, setEnableValidation] = React.useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nits:

Suggested change
const [isValidationEnabled, setEnableValidation] = React.useState(false);
const [isValidationEnabled, setIsValidationEnabled] = React.useState(false);

Comment on lines 62 to 66
setEnableValidation(true);
if (canJumpToNext) {
setStepIdReached(stepIdReached <= stepId ? stepId + 1 : stepIdReached);
onNext();
setEnableValidation(false);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
setEnableValidation(true);
if (canJumpToNext) {
setStepIdReached(stepIdReached <= stepId ? stepId + 1 : stepIdReached);
onNext();
setEnableValidation(false);
}
if (canJumpToNext) {
setStepIdReached(stepIdReached <= stepId ? stepId + 1 : stepIdReached);
onNext();
setEnableValidation(false);
} else setEnableValidation(true);

hasNoPaddingTop
isRequired
validated={
isValidationEnabled && !selectedLabels?.length ? 'error' : 'default'
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we create a small util function for mco ?
const getValidatedProp = (error: boolean) => error ? 'error' : 'default';

Copy link
Collaborator

@SanjalKatiyar SanjalKatiyar Sep 12, 2023

Choose a reason for hiding this comment

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

PF also have a constant variable/enum for 'error' & 'default', we can use that directly...


const isPVCSelectorFound = (dataPolicy: DRPolicyType) =>
!!dataPolicy?.placementControlInfo?.length &&
!!dataPolicy?.placementControlInfo?.every(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
!!dataPolicy?.placementControlInfo?.every(
!!dataPolicy.placementControlInfo.every(

Copy link
Collaborator

Choose a reason for hiding this comment

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

if !!dataPolicy?.placementControlInfo?.length is true then only we will reach to next condition... and if that is happening it means all fields are present...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

const isPVCSelectorFound = (dataPolicy: DRPolicyType) =>
!!dataPolicy?.placementControlInfo?.length &&
!!dataPolicy?.placementControlInfo?.every(
(drpc) => !!drpc?.pvcSelector?.length
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(drpc) => !!drpc?.pvcSelector?.length
(drpc) => !!drpc.pvcSelector?.length

Signed-off-by: Gowtham Shanmugasundaram <[email protected]>
@openshift-ci openshift-ci bot added the lgtm label Sep 13, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 13, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: GowthamShanmugam, SanjalKatiyar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@SanjalKatiyar
Copy link
Collaborator

/cherry-pick release-4.14

@openshift-cherrypick-robot

@SanjalKatiyar: once the present PR merges, I will cherry-pick it on top of release-4.14 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@SanjalKatiyar
Copy link
Collaborator

/cherry-pick release-4.14-compatibility

@openshift-cherrypick-robot

@SanjalKatiyar: once the present PR merges, I will cherry-pick it on top of release-4.14-compatibility in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.14-compatibility

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@@ -141,7 +128,7 @@ export const AssignPolicyView: React.FC<AssignPolicyViewProps> = ({
};
// assign DRPolicy
const promises = assignPromises(state.policy);
Promise.all(promises)
await Promise.all(promises)
Copy link
Collaborator

Choose a reason for hiding this comment

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

not needed... still not blocking the PR for this small nit...

Suggested change
await Promise.all(promises)
Promise.all(promises)

@openshift-merge-robot openshift-merge-robot merged commit 4cdc53a into red-hat-storage:master Sep 13, 2023
3 checks passed
@openshift-cherrypick-robot

@SanjalKatiyar: new pull request created: #1044

In response to this:

/cherry-pick release-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot

@SanjalKatiyar: new pull request created: #1045

In response to this:

/cherry-pick release-4.14-compatibility

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants