-
Notifications
You must be signed in to change notification settings - Fork 4
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
Integration of React-Select into Add Student, proper styling and cust… #554
Conversation
[diff-counting] Significant lines: 489. |
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.
Hey Desmond, the new features in this PR are super clean and visually look great! I attached a video of my test, I noticed two bugs:
Screen.Recording.2024-11-16.at.1.35.42.PM.mov
• The "Please select at least one need" error doesn't disappear after selecting more than one need.
• There is no address validation
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.
For address validation you can add this constant and then alter the address div:
const isAddress = (address: string) => {
let parsedAddr;
try {
if (address.includes(',')) {
parsedAddr = parseAddress(address);
} else {
parsedAddr = parseAddress(${address}, Ithaca, NY 14850
);
}
} catch {
return 'Invalid address';
}
const {
streetNumber,
streetName,
streetSuffix,
placeName,
stateName,
zipCode,
} = parsedAddr;
if (
!(
streetNumber &&
streetName &&
streetSuffix &&
placeName &&
stateName &&
zipCode
)
) {
return 'Invalid address';
}
return true;
};
@@ -178,11 +311,13 @@ const RiderModalInfo: React.FC<ModalFormProps> = ({ | |||
})} |
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.
add validate: isAddress,
under pattern
@@ -178,11 +311,13 @@ const RiderModalInfo: React.FC<ModalFormProps> = ({ | |||
})} | |||
type="text" | |||
aria-required="true" | |||
style={{ height: '60px' }} | |||
/> | |||
{errors.address && ( | |||
<p className={styles.error}>Please enter an address</p> |
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.
change to <p className={styles.errorMsg}>{errors.address.message}</p>
Summary
This pull request is the final fix for the student modal where we are able to integrate the react multi select component as well as adding custom needs and disabilities.
Test Plan
Manual Testing done. Data is cleaned properly before being sent to the database.
Notes
Screen.Recording.2024-11-15.at.2.57.13.AM.mov
Breaking Changes