-
Notifications
You must be signed in to change notification settings - Fork 4
[feat] 레이아웃 구현 #18
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
[feat] 레이아웃 구현 #18
Conversation
✅ Deploy Preview for thejulge1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
cozy-ito
left a comment
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.
고생하셨습니다. 👍
src/Router.tsx 라우트 파일에 적용까지 하면 좋을 것 같아요!
export const router = createBrowserRouter([
{
Component: AuthLayout,
children: authRoutes,
},
{
Component: MainLayout,
children: appRoutes,
},
]);아마 레이아웃 적용하면, required props를 전달받지 못해서 에러가 날 텐데요!
아래 리뷰에서처럼 굳이 props로 두지 않아도 될 거라고 생각해요 😅
이후 PR에서 전역 상태 및 여러 비동기 요청에 대한 로직으로 대신 처리하면 될 것 같습니다. 🤔
우선 페이지 작업을 위해서 Approved 먼저 드려요!
| isLoggedIn, | ||
| userNavLabel, | ||
| hasAlarm, | ||
| onLogout, | ||
| onToggleAlarm, |
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.
해당 props 들은 별도로 받지 않아도 될 것 같아요!
isLoggedIn, userNavLabel은 이후 로그인 후의 전역 상태에서 데이터를 받으면 될 것 같고,
hasAlarm, onToggleAlaam도 추후 알람을 위한 컴포넌트에서 처리,
onLogout도 로그아웃 버튼에서 바로 실행하면 될 것 같아요 🤔
#️⃣연관된 이슈
Closes #6
📝 PR 유형
📝작업 내용