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

Feaure/sidhantrohatgi/prescreeningquestions #32

Merged
merged 30 commits into from
Apr 23, 2024

Conversation

sidhantrohatgi
Copy link
Member

Tracking Info

Resolves #8

Changes

Implemented front end for the prescreening questions page (Page accessible by clicking on Apply in the Nav Bar)

Testing

  1. Compared how the page looks with the figma designs
  2. Tested the functionality of the paths (determined by user's answers) by trying different answer combinations as per the figma

Confirmation of Change

Page:

Screenshot 2024-03-12 at 21 10 39

Page with selected options:

Screenshot 2024-03-12 at 21 11 00

Redirected to this page according to the user's answers:

Screenshot 2024-03-12 at 21 11 12

@sidhantrohatgi sidhantrohatgi changed the title Feaure/prescreeningquestions Feaure/sidhantrohatgi/prescreeningquestions Mar 13, 2024
@Miyuki-L Miyuki-L requested review from jaredsbt and kathyychenn April 2, 2024 22:54
Copy link
Member

@Miyuki-L Miyuki-L left a comment

Choose a reason for hiding this comment

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

Great work!!!!! Just a few small things to change. I tried to link to all the necesary things and be specific about the changes. Let me know if there are still unclear things. Make sure to pull the updated changes from main as well.

frontend/src/App.tsx Outdated Show resolved Hide resolved
frontend/src/components/PrescreeningForm.tsx Outdated Show resolved Hide resolved
1. Please select the appropriate amount of experience you currently have.
</p>

<br></br>
Copy link
Member

Choose a reason for hiding this comment

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

Lets do self closing tags so <br /> for this one and all other <br> tags

frontend/src/components/PrescreeningForm.tsx Outdated Show resolved Hide resolved

const navigate = useNavigate();

const handleFormSubmit = (e: React.FormEvent<HTMLFormElement>) => {
Copy link
Member

Choose a reason for hiding this comment

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

Pull the code from main. Hrithik changes in button should work. The function would be
const handleFormSubmit = () => { ....} and the e.preventDefault(); won't be needed

Lets try to keep it consistent and just use our own button component

Copy link
Member

Choose a reason for hiding this comment

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

This file should be in the pages folder not the components folder

);
}

export default PrescreeningForm;
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this line here. We exported the function before and the convention in this project is to not use export default

@@ -5,5 +5,6 @@ import { NavBar } from "./NavBar.tsx";
import { Page } from "./Page.tsx";
import { Pathway } from "./Pathway.tsx";
import { PathwayTimeline } from "./PathwayTimeline.tsx";
import { PrescreeningForm } from "./PrescreeningForm.tsx";
Copy link
Member

Choose a reason for hiding this comment

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

This import should be in the index.ts file in the pages folder after you moved the files


<div className={styles.centeredContainer}>
<button className={buttonStyle.button} type="submit" form="form1" value="Submit">
Submit
Copy link
Member

Choose a reason for hiding this comment

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

Figma has this as continue instead of submit. Let's follow the figma

@@ -72,7 +72,7 @@ export function NavBar() {
</a>
</li>
<li>
<NavLink className={styles.navLink} to={"/apply"}>
<NavLink className={styles.navLink} to={"/prescreening"}>
<Button onClick={null}>Apply</Button>
Copy link
Member

Choose a reason for hiding this comment

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

There are also a few buttons on the home page here and here to update.

You can either use the useNavigate method you did in the handleSubmit or the window.location.href way used in the Congradulations popup

Copy link
Contributor

@jaredsbt jaredsbt left a comment

Choose a reason for hiding this comment

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

Nice work

Comment on lines 68 to 69
{/* Question 1 */}
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Each question is in div container. Label the div container and add more spacing between each question (marging top/bottom). Questions are a little too close to each other compared to Figma.
Screenshot 2024-04-15 at 12 36 54 PM


<label htmlFor="op11" className={styles.label}>
<p>
5+ years of Diversified Design Experience <strong> and/or </strong> 40+ Core Units
Copy link
Contributor

Choose a reason for hiding this comment

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

There is missing indentation for the radio points.

@Miyuki-L Miyuki-L requested review from jaredsbt and removed request for jaredsbt April 18, 2024 05:09
@Miyuki-L Miyuki-L merged commit b7b33b1 into main Apr 23, 2024
2 checks passed
@Miyuki-L Miyuki-L deleted the feaure/prescreeningquestions branch April 23, 2024 06:08
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.

Prescreening Question Screen
6 participants