feat: 어드민 구현 #373
feat: 어드민 구현 #373
Conversation
|
영상 녹화중에 발견된 백그라운드 범위, 어드민 메인 로고 페이드 등의 이슈는 추후 수정하겠습니다. |
ChangwooJ
force-pushed
the
283-feat-admin
branch
from
b2729c6 to
aafc1fa
Compare
|
작업량이 커서 다음에 작업할 때는 기능 브랜치 올리고 이슈 쪼개고 거기에 머지하는 pr 잘게 올리면 좋을 것 같습니다! |
sins051301
left a comment
sins051301
left a comment
There was a problem hiding this comment.
코드 작성 시에 변수명이나 함수명 지을 때 신경써보면 좋을 것 같아요
| dispatch({ type: 'VALIDATE_FIELD', name }); | ||
| }; | ||
|
|
||
| const isStep1Valid = () => { |
There was a problem hiding this comment.
isStep1Valid라는 변수명보다는 이 단계에서 뭘하는지 설명해주는 변수 명이면 좋을 것 같아요
There was a problem hiding this comment.
넵 이거 상징적인 의미로 만들어서 명명해보겠습니다!
| }); | ||
| }; | ||
|
|
||
| const isStep2Valid = () => { |
| return ( | ||
| <form | ||
| className="flex flex-col gap-2 py-8" | ||
| onSubmit={async (e) => { |
There was a problem hiding this comment.
위에서 함수로 묶어서 처리하는게 가독성과 추후 확장성을 위해 좋을 것 같아요
There was a problem hiding this comment.
그렇겠네요 지금도 충분히 길어서 분리해두겠습니다
| onBlur: (field: keyof RecruitmentFormData) => void; | ||
| } | ||
|
|
||
| function StepContentRecruitment({ |
There was a problem hiding this comment.
글작성 부분을 나타내는 변수명으로 더 직관적이게 지으면 좋을 것 같아요
There was a problem hiding this comment.
모집글/소개글 or 글생성/수정및삭제 이렇게 구성되어 있어서 네이밍이 조금 모호한거 같아요..
혹시 도움을 조금 주실 수 있나요?
Target이나 Subjet
Mode나 Task
이런식으로도 생각해봤는데 잘맞는 네이밍이 생각나지 않네요..
| @@ -0,0 +1,48 @@ | |||
| import Textarea from '@/shared/ui/textarea'; | |||
| import { RecruitmentFormData } from '../../model/type'; | |||
|
|
|||
There was a problem hiding this comment.
프롭스 타입도 함수명에 따라 비슷하게 통일해주세요
There was a problem hiding this comment.
Step까지 붙이는게 낫군요!
해당 Prop의 의미가 Step을 포함하진 않는거 같아서 제외했었습니다 포함해두겠습니다
| </Button> | ||
| )} | ||
| </> | ||
| )} |
There was a problem hiding this comment.
https://www.youtube.com/watch?v=NwLWX2RNVcw
퍼널 설계인 거 같은데 이거 참고하면 좋을 것 같아요
There was a problem hiding this comment.
너무 좋은 자료 감사합니다.
퍼널 설계인데 뒤에 라이브러리화가 인상 깊네요.
지금처럼 설계해도 문제는 없지만 지원폼이라는 특성 상 또 사용될 일이 있을거 같고, 흩어져있는 것도 유지보수에 좋지 않으니 한번 개선해보겠습니다.
| }; | ||
| } | ||
|
|
||
| export interface RecruitmentPatchResponse { |
There was a problem hiding this comment.
export type RecruitmentPatchResponse = RecruitmentResponse;
이런식으로 선언하면 중복으로 작성안해도 될 것 같아요
| }; | ||
| } | ||
|
|
||
| export interface RecruitmentDeleteResponse { |
There was a problem hiding this comment.
아니면 제네릭으로 처리해도 방법일 것 같아요
// 공통 베이스 구조
interface BaseResponse {
ok: boolean;
message: string;
data?: T; // 알맹이만 제네릭으로 처리
}
// 1. 생성 및 수정용 데이터 구조
export interface RecruitmentData {
data: {
id: number;
uploadImageUrls: string[];
};
}
// 2. 삭제용 데이터 구조
export interface RecruitmentDeleteData {
id: number;
deleteImageUrls: string[];
}
// 최종 타입 정의
export type RecruitmentResponse = BaseResponse;
export type RecruitmentPatchResponse = RecruitmentResponse; // 완전 동일하므로 별칭 사용
export type RecruitmentDeleteResponse = BaseResponse;
There was a problem hiding this comment.
확장성을 생각하면 이 방식이 더 좋아보이네요! 적용하겠습니다.
| case 'VALIDATE_FIELD': { | ||
| const { name } = action; | ||
| const value = state.formData[name]; | ||
| const errorMsg = validateField(name, value as string); |
There was a problem hiding this comment.
validateField라는 변수명인데 에러를 반환하는게 이상한거 같아요
getValidationError 같은 변수명은 어떤가요?
| onDragOver, | ||
| onDrop, | ||
| } = useImageUpload( | ||
| action === 'edit' && initialData ? initialData.imageUrls : [], |
There was a problem hiding this comment.
훅안에서 선언하는 것보다 밖에서 선언하고 주는 것이 더 직관적일 것 같아요
There was a problem hiding this comment.
따로 분리하여 적용해두겠습니다.
|
좋은 리뷰 감사합니다. |
#️⃣연관된 이슈
[#283 ] feat: 관리자 페이지 분리
📝작업 내용
관리자 페이지 분리
스크린샷 (선택)
-.Clipchamp.mp4
💬리뷰 요구사항(선택)
단계별 폼 제출이 가능하도록 변경했습니다.
현재 개발 서버 사용이 불가능한 상태라 테스트는 진행되지 않았습니다.
추가적으로 동아리 소개 부분은 기존 방식과 상이한 점이 존재하는데 주요한 문제로는 api를 동기적으로 2가지 사용해야한다는 점입니다.
(동아리 등록 -> 동아리 정보 수정)
이 부분에 대해서는 추후 정기 회의 이후 재반영하겠습니다. (PR 닫지 말아주세요!)
오버레이 부분은 AI를 적극 활용해 수정하였습니다.