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

Feature/madhav/step3 #51

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Feature/madhav/step3 #51

wants to merge 10 commits into from

Conversation

sidhantrohatgi
Copy link
Member

Tracking Info

Resolves #27

Changes

Made a Step3 component for Work Experience and added it to the paths

Testing

Ran the frontend and checked if the changes were reflected according to the figma

Confirmation of Change

image image

Made Temporary path called Step 3 for visualization, Added WorkExperienceForm and Step 3 Components.
@Miyuki-L Miyuki-L requested a review from kathyychenn May 7, 2024 23:27
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.

Good work. A few changes, For most of the changes you can reference Step 1/2 that is already in the main branch.

{add ? (
<div className={styles.nameWithAdd}>
<h3 className={styles.sectionName}>{sectionName}</h3>
<button className={styles.add}>
Copy link
Member

Choose a reason for hiding this comment

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

give this type: button so that it doesn't trigger the submit for the form.

Also add in the functionality for the add. You can take a look at Step 2 to see how Kathy and Emma implemented the Add.

Missing the delete button.

Also make it such that for required fields the delete button doesn't show up for the first 1.

You can reference step 1 and 2 for all these delete and add functionality.

Copy link
Member

Choose a reason for hiding this comment

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

Work experience isn't required for path 1 so you'll need to implement that Step 2 Schools section has implemented a similar functionality as School isn't required for path 1.

</div>
<hr />
<div className={styles.formSection}>
<WorkExperienceForm
Copy link
Member

Choose a reason for hiding this comment

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

This section is missing the red asteriks when required

formInputs={[
{
inputTitle: "Recent diversified design experience",
defaultMessage: "Design Experience",
Copy link
Member

Choose a reason for hiding this comment

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

place holder text for every field in the Work Experience as "Enter" at the begining.

required: true,
},
{
inputTitle: "How many hours per week did you work (on average)?",
Copy link
Member

Choose a reason for hiding this comment

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

The input field ordering of company name, howers per week, and phone number is different


.sectionName {
margin: 0;
font-size: 18px !important;
Copy link
Member

Choose a reason for hiding this comment

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

Is there really a need for !important?

border-radius: 5px;
}

/* .inputTextSelected {
Copy link
Member

Choose a reason for hiding this comment

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

Remove commented code

background-repeat: no-repeat;
background-position: right top 50%;
cursor: pointer;
/* color: var(--color-accent); */
Copy link
Member

Choose a reason for hiding this comment

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

came remove if not using

{formInput.inputType === "dropdown" &&
formInput.dropdownOptions &&
(formInput.required ? (
<select className={styles.inputText} id="drop">
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 the select stuff. THere is no drop down for this page

Copy link
Member

Choose a reason for hiding this comment

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

The section seems to be missing some border or shadow on the outer most

The edge between the form and the background

image

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.

Step 3: Work Experience
3 participants