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

feat: 지원서 질문 업데이트 #176

Merged
merged 13 commits into from
Sep 1, 2024
Merged

Conversation

geongyu09
Copy link
Collaborator

@geongyu09 geongyu09 commented Aug 31, 2024

주요 변경사항

아래의 요구사항이 있었고, 면접 시간 변경과 관련한 부분을 제외한 모든 부분을 업데이트 완료했습니다.

1. 개인정보 수집 안내문 날짜 업데이트

이전에 따로 진행한 부분입니다.
#170

2. 주간발표 문구 수정

image

3. 포트폴리오 업데이트 부분 경고 문구 추가

요구사항에 맞추어 문구추가 + 볼드체 + 밑줄을 하기 위해 기존과는 다른 형식이 필요하였고, alert라는 프로퍼티를 옵셔널로 추가하여 해당 부분이 있다면 보여주도록 변경하였습니다.
image

4. 기타 질문들 업데이트

가. 본인이 계획하고 있는 진로가 무엇인가요? (디자이너, pm) ->본인이 계획하고 있는 진로와 이를 위해 노력한 내용을 말씀해주세요
image

나. 디자이너 필수 질문에 *이 붙어있지 않는 부분 수정
(사진 생략)

다. 면접 가능 시간 경고 문구 추가
image

리뷰어에게...

  • 오탈자나 코드 변경으로 인한 사이드 이팩트가 있을지 확인 부탁드립니다! ( 일어날 수 있을만한 유저 플로우를 전부 수기로 테스트 해봤지만, 발견하지 못했습니다! )
  • 추가적으로 이번에 마지막 면접 시간(20:30)이 30분으로 끝나게 되어 마지막 부분에 시간이 표시되지 않습니다(아래 사진 참고). 약간은 어색한 부분도 있고 유저 입장에서 불친절한 느낌도 들 수 있을 것 같아서 마지막 20:30분을 표시하는 것에 대해서는 어떻게 생각하시는지 궁금합니다..! 뭔가 추가하면 디자인적으로 안예뻐지는 느낌도 들긴 합니다..
마지막 시간(20:30) 표시 전후 비교하기

마지막 타임라인에 시간이 없어서 유저 입장에서는 20:30 까지일 것으로 추측 해야함
image

만약 마지막 타임라인에 시간을 표시하게 된다면 유저 입장에서 친절하지만 예쁘지 않은(?) 느낌
image

@geongyu09 geongyu09 requested review from 2yunseong and smb0123 August 31, 2024 16:47
@geongyu09 geongyu09 self-assigned this Aug 31, 2024
@geongyu09 geongyu09 force-pushed the feat/175-update-application branch from 11ad975 to 5a4e3d2 Compare August 31, 2024 16:48
@2yunseong
Copy link
Collaborator

짧은 시간안에 ... 고생하셨어요 ㅠ
저는 이쁜것보다 명확한게 낫다는 의견입니다!

너무 고생하셨고, 지금은 밖이라 대강보았는데 집가서 리뷰 남기도록 하겠습니다~

Copy link
Collaborator

@2yunseong 2yunseong left a comment

Choose a reason for hiding this comment

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

건규님 하나만 설명부탁드립니다!

Comment on lines 182 to 190
endTime: new Date(2024, 9, 23, 20, 30, 0),
},
{
startTime: new Date(2024, 9, 24, 10, 0, 0),
endTime: new Date(2024, 9, 24, 21, 0, 0),
endTime: new Date(2024, 9, 24, 20, 30, 0),
},
{
startTime: new Date(2024, 9, 25, 10, 0, 0),
endTime: new Date(2024, 9, 25, 21, 0, 0),
endTime: new Date(2024, 9, 25, 20, 30, 0),
Copy link
Collaborator

Choose a reason for hiding this comment

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

monIndex=0 이 1월입니다. 확인한번 해주세요!
https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/Date/Date

Copy link
Collaborator Author

@geongyu09 geongyu09 Sep 1, 2024

Choose a reason for hiding this comment

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

수정 완료했습니다!
인지는 하고 있었지만, 9로 해두어야지만 9월로 표시가 되었고, 또 26.th 에도 9로 되어있더라고요..
어디선가 사이드 이팩트가 존재한다고 느꼈고, 해당 상수값(9로 설정된 Date 객체)을 다른 곳에서 사용중이라면 9를 8로 고치는 것보다 잘 돌아가는 시스템이니 9로 두는 것이 더 올바르다고 생각했던 것 같습니다.

확인해 보니 실제로는 다른 곳에서 사용을 하지 않고, 면접 시간 입력 페이지에서 `${ .getmonth()}월`로 표시만 하여서 그랬던 것이며, 단순히 `${ .getmonth() + 1}월` 형태로 변경만 하여 해결 완료했습니다!

너무 회피형이었네요... 다음에는 문제를 직면했을 때 그 본질을 해결하려고 노력해 보겠습니다!

@geongyu09 geongyu09 requested a review from 2yunseong September 1, 2024 02:10
Copy link
Collaborator

@loopy-lim loopy-lim left a comment

Choose a reason for hiding this comment

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

어.... 죄송합니다 ㅠㅠ 제 실수입니다....하하....
추가적으로 먼가 더 조취를 취하는 것이 있으면 좋을만한 것을 comment으로 남겨두었습니다.
항상 감사합니다

Comment on lines +26 to +32
{applicationQuestion.alert && (
<div className="mt-4">
<Txt className=" underline font-semibold">
⚠️ {applicationQuestion.alert}
</Txt>
</div>
)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

먼가 이걸 보니깐 나중에 된다면, component으로 빼서 관리해도 좋겠다라는 생각이 드네요

Copy link
Collaborator Author

@geongyu09 geongyu09 Sep 1, 2024

Choose a reason for hiding this comment

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

넵! 이 로직이 이미 2번이나 반복이 되어버려서 컴포넌트로 빼보겠습니다

className="block pb-8"
>{`${startTime.getMonth()}월 ${startTime.getDate()}일`}</Txt>
<Txt typography="h6" className="block pb-8">{`${
startTime.getMonth() + 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅎㅎ.... 죄송합니다...ㅋㅋㅋㅋ
사실 javascript에서 date는 항상 애매한 문제가 있어서, data-fns와 같은 라이브러리를 사용하는 것을 추천합니다.(저도 최근에서야 좀 도입을 시작했네요 ㅠㅠ)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

헉 아니에요!!
data-fns도입도 고려해보도록 하겠습니다!

>{`${startTime.getMonth()}월 ${startTime.getDate()}일`}</Txt>
<Txt typography="h6" className="block pb-8">{`${
startTime.getMonth() + 1
}월 ${startTime.getDate()}일 (${convertDay(startTime.getDay())})`}</Txt>
Copy link
Collaborator

Choose a reason for hiding this comment

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

어...제가 싼 똥이지만 이를 다시 바로 잡기 위해서는 toLocaleDateString()이라는 함수를 사용해도 좋을 것 같다는 생각이 듭니다.
문서는 여기에 있습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

와우 이저 정말 유용한데요?! 바로 적용해보겠습니다

디자이너 혹은 기획자를 눌렀을 때 변경되기 전의 질문이 보이는 버그 수정
27로 불러오는 부분을 전부 28로 변경하였습니다

#175
Copy link
Collaborator

@smb0123 smb0123 left a comment

Choose a reason for hiding this comment

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

굿굿 수고하셨어요 !

@geongyu09 geongyu09 merged commit b112ae4 into main Sep 1, 2024
1 check passed
@geongyu09 geongyu09 deleted the feat/175-update-application branch September 1, 2024 08:39
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.

4 participants