Skip to content

Conversation

@jmgasper
Copy link
Collaborator

Related JIRA Ticket:
https://topcoder.atlassian.net/browse/MP-75

What's in this PR?

New onboarding app moving into dev

suppermancool and others added 30 commits June 22, 2023 18:39
- onboarding app page 2/7
Onboarding: work experience page and education page
onboarding: "+" button is disable for new user
- Next button on the Onboarding step 1 is not working
The entered data is not saved if the user clicks Back button in any step
Update onboarding to use new skill selector component
jmgasper and others added 13 commits July 5, 2023 13:32
# Conflicts:
#	src/apps/profiles/src/config/constants.ts
#	src/apps/profiles/src/member-profile/community-awards/CommunityAwards.module.scss
#	src/apps/profiles/src/member-profile/education-and-certifications/EducationAndCertifications.module.scss
#	src/apps/profiles/src/member-profile/education-and-certifications/EducationCard/EducationCard.tsx
#	src/apps/profiles/src/member-profile/education-and-certifications/ModifyEducationModal/ModifyEducationModal.tsx
#	src/apps/profiles/src/member-profile/languages/MemberLanguages.tsx
#	src/apps/profiles/src/member-profile/page-layout/ProfilePageLayout.module.scss
#	src/apps/profiles/src/member-profile/page-layout/ProfilePageLayout.tsx
#	src/apps/profiles/src/member-profile/profile-header/ProfileHeader.module.scss
#	src/apps/profiles/src/member-profile/profile-header/ProfileHeader.tsx
#	src/apps/profiles/src/member-profile/skills/MemberSkillsInfo.module.scss
#	src/apps/profiles/src/member-profile/skills/MemberSkillsInfo.tsx
#	src/apps/profiles/src/member-profile/tc-achievements/MemberTCAchievements.tsx
#	src/apps/profiles/src/member-profile/work-expirence/ModifyWorkExpirenceModal/ModifyWorkExpirenceModal.tsx
#	src/apps/profiles/src/member-profile/work-expirence/WorkExpirence.module.scss
#	src/apps/profiles/src/member-profile/work-expirence/WorkExpirence.tsx
#	src/apps/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.tsx
#	src/apps/profiles/src/profiles.routes.tsx
#	src/libs/ui/lib/components/form/form-groups/form-input/input-date-picker/InputDatePicker.tsx
#	yarn.lock
- Exiting out of onboarding flow takes the user to their profile
@jmgasper jmgasper requested a review from kkartunov July 10, 2023 16:19
Copy link
Collaborator

@kkartunov kkartunov left a comment

Choose a reason for hiding this comment

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

Looks good!

@vas3a vas3a requested review from kkartunov and vas3a July 10, 2023 16:46
Copy link
Collaborator

@vas3a vas3a left a comment

Choose a reason for hiding this comment

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

I'd suggest sticking with the eslint rules present into the codebase rather than disable them in every file. This brings inconsistency and was hoping to see some clean code if this is freshly implemented.

@@ -0,0 +1,3 @@
{
"vue.features.codeActions.enable": false
Copy link
Collaborator

Choose a reason for hiding this comment

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

should remove this file probably

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, logging this, thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

for icons, any reason for not using the HeroIcons? Some of these SVGs are matching the heroicons which are already included in the repo.

@@ -0,0 +1,97 @@
/* eslint-disable ordered-imports/ordered-imports */
Copy link
Collaborator

Choose a reason for hiding this comment

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

why all the eslint disable? is this DateInput component ported from somewhere else?
I know @kkartunov also has worked on a date input component for profile, could they be merged/reused?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vas3a i only see one date input component in src/libs/ui/lib/components/form/form-groups/form-input/input-date-picker/InputDatePicker.tsx. I can not use that component for some reasons:

  • That component does not match with onboarding design
  • That component does not work well inside BaseModal (from '~/libs/ui')

Copy link
Contributor

Choose a reason for hiding this comment

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

@jmgasper should I use the existing component even if it does not match with the design?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@suppermancool - I'll let you know, but we can pause on this one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jmgasper @suppermancool There's no need to use it if it's too different or not compatible. Just want to make sure we're not adding components if we can use existing ones. thanks!

@@ -0,0 +1,143 @@
/* eslint-disable ordered-imports/ordered-imports */
/* eslint-disable react/jsx-no-bind */
/* eslint-disable react/destructuring-assignment */
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, why all the eslint disables?

primary
size='lg'
label='save'
onClick={() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should move the onclick handler into a named/callback function.

name='company'
label='Company *'
value={workInfo.company}
onChange={event => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here.

@jmgasper jmgasper mentioned this pull request Jul 10, 2023
Copy link
Collaborator

@vas3a vas3a left a comment

Choose a reason for hiding this comment

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

Looks better! 🤞 thanks!

@jmgasper jmgasper merged commit 98a1acf into dev Jul 10, 2023
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.

5 participants