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] org admin 빈 곳 있을 시 해당 섹션으로 이동 #179

Merged
merged 14 commits into from
Dec 31, 2024

Conversation

eonseok-jeon
Copy link
Member

@eonseok-jeon eonseok-jeon commented Dec 26, 2024

✅ PR Point

더 효율적인 방법이 없을까 고민을 많이 했는데 찾지 못했어요
일일이 다 값 비교해보고 빈 값 있으면 해당 섹션으로 이동하게 했어요
홈 탭은 아직 주용이 pr 머지 안 되어서 일단 그대로 뒀어요

일단 현재 페이지에 빈 곳이 있는지 없는지 체크를 먼저하게 했어요
특히 Chip을 이용하는 곳은 빈 곳이 있을 경우 해당 Chip이 선택 되도록 했어요
여러 곳이 비어있다면 그 중 제일 첫 번째 input만 error 처리 되도록 했어요
다 채워져 있다면 공통 -> 홈 -> 소개 -> 지원하기 탭 순으로 체크하게 했습니다

추가로 빈 필드가 있을 시 error toast가 뜨도록 구현했어요
스크린샷 2024-12-27 오후 10 45 35

앞으로 해야할 것

  • 수정하기 탭 input들 에러 상태 UI 처리
  • 홈 탭 input들 validate 추가

@eonseok-jeon eonseok-jeon self-assigned this Dec 26, 2024
Copy link

height bot commented Dec 26, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link

cloudflare-workers-and-pages bot commented Dec 26, 2024

Deploying sopt-admin with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2890184
Status: ✅  Deploy successful!
Preview URL: https://089d0921.sopt-admin.pages.dev
Branch Preview URL: https://feature-org-common-fill-yb.sopt-admin.pages.dev

View logs

Copy link
Member

@lydiacho lydiacho left a comment

Choose a reason for hiding this comment

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

너무너무 고생많으셨습니다 😭😭

참 마음이 아픈 코드네요 보면서 뭔가 애초에 Form을 탭별로 분리해서 관리해주고, 배포 버튼에는 따로 부모 form 만들어서 value들 묶어서 submit하도록 설계했어야하나.. 싶기도하고...

무튼 고민하느라 고생많으셨습니다ㅠ! 코멘트 몇개만 한번 읽어봐주세요

src/components/org/OrgAdmin/index.tsx Outdated Show resolved Hide resolved
Comment on lines +23 to +36
...['OB', 'YB'].flatMap((group) =>
[
'applicationStartTime',
'applicationEndTime',
'applicationResultTime',
'interviewStartTime',
'interviewEndTime',
'finalResultTime',
].map((time) => ({
name: `recruitSchedule.${group}.${time}`,
value: recruitSchedule?.[group]?.[time],
})),
),
...['main', 'low', 'high', 'point'].map((color) => ({
Copy link
Member

Choose a reason for hiding this comment

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

요런 배열 값들 하드코딩하기보단 어차피 form item들이랑 일대일대응될 테니까 스키마에서 배열 뽑아와도 될 수 것 같기도 하네욤

Copy link
Member Author

Choose a reason for hiding this comment

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

오홍 좋은 거 같아요! 시도해보고 말씀드릴게요 :)

src/components/org/OrgAdmin/utils.ts Outdated Show resolved Hide resolved
validationCommonInputs(getValues, setError, setGroup);
const handleValidationAboutInputs = () =>
validationAboutInputs(
getValues,
Copy link
Member

Choose a reason for hiding this comment

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

얘네 getValues는 함수 자체를 넘겨주는 방식 외에도, handler 내에서 getValues로 해당 validation에 필요한 필드값들 뽑고, 걔네를 객체로 묶어서 전달해주는 것도 깔끔할 것 같은데 어떻게 생각하시나욤?

Copy link
Member Author

Choose a reason for hiding this comment

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

아ㅏㅏ 그래도 되는데 그러면 해당 객체에 대한 타입들 또 지정해줘야 해서 고냥 바로 getValues 넘겨줬습니다

통일성도 있고 나쁘지 않은 거 같아서요!

Copy link
Member

@wuzoo wuzoo left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 !!

src/components/org/OrgAdmin/utils.ts Outdated Show resolved Hide resolved
@eonseok-jeon eonseok-jeon changed the base branch from refactor-org-register-naming to dev December 31, 2024 11:58
@eonseok-jeon eonseok-jeon merged commit 0580ee8 into dev Dec 31, 2024
1 check passed
@eonseok-jeon eonseok-jeon deleted the feature-org-common-fill-yb branch January 12, 2025 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants