Skip to content

Conversation

@grapefruit13
Copy link
Contributor

유형

  • 기능 구현
  • UI 구현
  • 리팩토링
  • 버그 해결
  • 문서 업데이트
  • 기타( )

작업 내용

  • NavProfile 리디자인 적용 및 Header 구현

설명

  • 스크린 샷에서는 제일 위에 header만 확인해보시면 됩니다
  • 추후에 프로필 이미지 다른 것으로 바꾸는거 까먹지 않도록 주석 달아두었습니다

스크린샷

image

리뷰 요구사항

  • 디스코드에도 말씀드렸지만 typo 컴포넌트 만들 때 아래처럼 type이랑 color명 넣는 것 어떤가요?? 기존에는 해당 타이포가 사용되는 컴포넌트명을 사용해서 만들었었는데, 그 타이포가 다른 컴포넌트에도 쓰이는 경우가 있어서요~
export const Subtitle2Black = customTypography('span', {
  type: 'Subtitle2',
  color: 'black',
});

@grapefruit13 grapefruit13 self-assigned this Jan 12, 2025
@grapefruit13 grapefruit13 requested a review from plla2 January 12, 2025 13:52
Copy link
Member

@plla2 plla2 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 9 to 14
const [isMounted, setIsMounted] = useState(false);
const username = useUserStore((state) => state.username);

useEffect(() => {
setIsMounted(true);
}, []);
Copy link
Member

Choose a reason for hiding this comment

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

저희 isMounted 처럼 clientonly에서 사용할 훅 커스텀훅으로 빼도 좋을 것 같습니다!
chart 같은 브라우저 전용에서나 window객체를 사용할 때같이 필요할 때가 있을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 분리해서 커밋 남기겠습니다~

Comment on lines +46 to +50
export const Subtitle2Black = customTypography('span', {
type: 'Subtitle2',
color: 'black',
});

Copy link
Member

Choose a reason for hiding this comment

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

오늘 작업하면서 이부분처럼 customTypography 선언한 부분 변경해두겠습니다

@grapefruit13 grapefruit13 merged commit d30bcba into dev Jan 13, 2025
@grapefruit13 grapefruit13 deleted the THKV-119 branch January 13, 2025 02:30
plla2 added a commit to plla2/NNplanner-FE that referenced this pull request Jan 13, 2025
Feat[#THKV-119] : NavProfile 리디자인 적용 및 Header 구현
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.

3 participants