-
Notifications
You must be signed in to change notification settings - Fork 3
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 : 공통컴포넌트 - 헤더 및 레이아웃 작업 #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 ~
리뷰 한번씩 확인 부탁해요 !
혹시 긴히 논의할 사항이 있으면 슬랙 DM으로 언제든지 말씀해주세용
{children} | ||
<body className="flex h-[100dvh] w-screen touch-none justify-center bg-slate-100 py-px"> | ||
<div className="max-w-440 h-full w-full bg-white py-2 text-black drop-shadow-2xl"> | ||
<Header /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기에 <Header />
를 두게되면 모든 페이지에 공통으로 이 <Header />
가 적용이 될거에요.여기에 두는 것보다는 각 폴더의 layout에 두는 것이 더 적합하다고 생각해요 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앱라우팅이 처음이라 잘 몰라서 일단 다른 프로젝트들이 네비게이션이나 헤더는 공통 레이아웃에 두는 것 같아 따라해봤습니다 ㅎㅎ...! 각 폴더의 레이아웃에 두면 오히려 중복이 많아질것 같아서요..
피그마에는 모든 경로에 이 헤더가 있는 것 같아서.. + 서브 헤더가 필요한 경우에는 라우팅으로 구분하려고 했는데 이 방법은 너무 비효율적일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
최상단 layout.tsx
는 현재 server component여서 pathname을 받아오지 못합니다.
- 참고 링크 : [Next 13] Server Component + Layout.tsx - Can't access the URL / Pathname vercel/next.js#43704
따라서, path에 따라 다른 Header를 보여줄 수 없습니다.
leerob.io와 같은 방식으로 Header에서 다르게 보여주는 방법도 있지만, 저희는 포즈픽,톡,피드 외의 다른 페이지와 Header구성이 전혀 다르기에 적합한 방식이라 생각하지 않습니다.
묶어서 layout을 적용할 수 있는 Route Handler를 이용하면 좋을 것 같습니다. 이 부분은 제가 한 번 해볼게요 !
src/components/header/Tab.tsx
Outdated
<div key={item.path}> | ||
<div className="py-3"> | ||
<h5 className="text-brand">{item.title}</h5> | ||
</div> | ||
<div className="border-b-main-violet relative top-0.5 border-b-2" /> | ||
</div> | ||
) : ( | ||
<Link href={item.path} className="py-3" key={item.path}> | ||
<h5 className="text-secondary">{item.title}</h5> | ||
</Link> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 py-3
, key={item.path}
코드가 중복되고 있습니다.
이러한 코드를 묶어주면 더 가독성이 있고, 유지보수 측면에서도 좋을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{tabData.map((item) => (
<div key={item.path} className="py-3">
{item.path === path ? (
<h5 className="text-brand">{item.title}</h5>
) : (
<Link href={item.path}>
<h5 className="text-secondary">{item.title}</h5>
</Link>
)}
</div>
))}
제가 짜본 코드입니다. 참고 부탁해요 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tailwind는 중복 처리에 불리하다고 생각했는데 규성님처럼 하면 해결되겠군요..! 감사합니다 적용하겠습니다!
또한, 저희 commit할 때 이모지 넣는 것에 대해서도 논의가 필요해 보여요 ! |
💡 왜 PR을 올렸나요?
💁 무엇이 어떻게 바뀌나요?
📆 작업 예정인 것이 있나요 ?
💬 리뷰어분들께