Feat/mypage 수정 #30
Feat/mypage 수정 #30
Conversation
- file onChange 수정 - radio : boolean으로 수정
🦋 Changeset detectedLatest commit: ced30bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
TODO
|
hongggyelim
force-pushed
the
feat/mypage
branch
from
b6791a2 to
744c2ec
Compare
|
피그마에 정보수정 페이지 내 모달 너비 420 px 가 아님 - 컴포넌트 자체는 420px fixed 라서 확인 부탁드립니다 |
jongjunpark
left a comment
jongjunpark
left a comment
There was a problem hiding this comment.
파일/폴더명 컨벤션 및 아키텍쳐 통일성필요
- 같은 단어이나 컨벤션이 다른 케이스
- app/mypage
- shared/components/myPage
- 같은 dir 내에 다른 컨벤션
- shared/components/myPage/Stages (이것만 pascal)
- shared/components/myPage/components
- 아키텍쳐의 모호성
- shared/components/myPage/components (components에 또 다시 components가 등장)
- 3번과 비슷 구조적으로 어떻게 나뉜거인지 잘 모르겠음
아래 두개 종류로 나뉜것 같은데, 어떤 기준점인지 모호함- shared/components/layouts
- shared/components/myPage, notApproved, sigupSection
- style 파일 위치가 제각각
왜 mypage만 따로 있는지 모르겠음 그러나 개인적으로 mypage 처럼 나뉘었으면 좋겠음 하나의 파일에서 관리하는건 좋은 것은 아님- (auth)/_styles/style.css.ts
- (auth)/mypage/style.css.ts
- libs의 역할
libs는 libarary를 의미하나 파일들이 모두 상수만 있음. 이는 model, constants, config와 같은 네이밍이 더 어울림
apps/user/src/app/my-page/page.tsx
Outdated
| {stage === "required" && <RequiredStage />} | ||
| {stage === "optional" && <OptionalStage />} | ||
| {stage === "account" && <AccountStage />} |
There was a problem hiding this comment.
요런거 ts-pattern 같은 라이브러리 써보는것도 추천
| }, [isOpenDropdown]); | ||
|
|
||
| useEffect(() => { | ||
| setHeaderDOM(document.getElementById("main")); |
There was a problem hiding this comment.
apps/user/src/app/(auth)/layout.tsx가 적용되는 페이지가 login, signup인데 header를 쓰지않는 페이지인데,
왜 여기에 state값을 저장하려는건지 궁금함
There was a problem hiding this comment.
추가로 DOM을 가져오는건 지양하고 ref를 활용했으면 함
| /> | ||
| )} | ||
| {isOpenDropdown && | ||
| createPortal( |
There was a problem hiding this comment.
Portal 컴포넌트를 만들었으니깐 만든걸 활용
| @@ -1,6 +1,5 @@ | |||
| import { Color, textSprinkles } from "@repo/ui"; | |||
| import { Color, textSprinkles, Zindex } from "@repo/ui"; | |||
There was a problem hiding this comment.
요거 왜 zIndex가 아니라 Zindex로 이상하게 되었지
| @@ -16,6 +18,8 @@ import { | |||
| * @todo : 입력한 정보가 있을때는 해당 데이터를 가져와야함 | |||
| */ | |||
| const AccountStage = () => { | |||
| const [isOpenResetPasswordDialog, setIsOpenResetPasswordDialog] = | |||
| useState(false); | |||
| <Dialog.Content> | ||
| <div className={content}> | ||
| <Label>현재 비밀번호</Label> | ||
| <TextInput |
There was a problem hiding this comment.
TextInput에 type="password" 추가 필요
| </div> | ||
| </Dialog.Content> | ||
| <Dialog.Footer> | ||
| <Button type="button" variant="outline" onClick={onClose}> |
There was a problem hiding this comment.
Button의 높이가 디자인과 다름
- Button 디자인이 기본 40px 고정값을 사용하고 있음 참고
There was a problem hiding this comment.
디자인과 상이한 부분
- Dialog size 설정필요 (참고)
- label gap이 디자인과 다름
There was a problem hiding this comment.
tooltip은 tool-tip 합성어가 아니라 tooltip이 하나의 단어임
따라서, ToolTip이 아니라 Tooltip이 맞음
| }; | ||
| const ToolTip = ({ content }: Props) => { | ||
| return ( | ||
| <> |
추가 리뷰
|
squash
hongggyelim
force-pushed
the
feat/mypage
branch
from
a2a209a to
ced30bc
Compare
마이페이지 UI 변경사항 반영 [4/29]
비밀번호 변경 모달 추가 [10/8]