-
Notifications
You must be signed in to change notification settings - Fork 50
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
강원대_FE_허윤수_2주차_과제 #60
base: sugoring
Are you sure you want to change the base?
강원대_FE_허윤수_2주차_과제 #60
Conversation
- 라우팅 관리를 위해 react-router-dom 추가
- `react-router-dom`의 RouterProvider를 사용하여 라우터 설정
- Header 컴포넌트를 애플리케이션 상단에 추가 - react-router-dom의 Outlet을 사용하여 동적 라우팅 구현 - Footer 컴포넌트를 애플리케이션 하단에 추가
- createBrowserRouter를 사용하여 라우터 설정 구성 - 각 경로에 따라 App 컴포넌트가 최상위에 렌더링되도록 설정 - Routes 컴포넌트를 통해 각 경로별 페이지 컴포넌트 매핑
import Title from '@/components/Header/Title'; | ||
|
||
interface HeaderProps { | ||
themeKey?: string; |
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.
옵셔널로 설정하신 이유가 있을까요?
<Title>Hello, {name}</Title> | ||
</div> | ||
<AuthProvider> | ||
<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.
여기서 themeKey를 넣어주지 않고 있는 것으로 보이는데요~
import type { Item } from '@/pages/MainPage/types'; | ||
|
||
const MainPage = () => { | ||
const items: Item[] = getItems(); |
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.
Item[]으로 명시할 필요는 없어보여요~
import { useState } from 'react'; | ||
|
||
export const useCount = (initialCount: number) => { | ||
const [count, setCount] = useState(initialCount); | ||
|
||
const handleIncrease = () => { | ||
setCount((prevCount) => prevCount + 6); | ||
}; | ||
|
||
const handleReset = () => { | ||
setCount(6); | ||
}; | ||
|
||
return { | ||
count, | ||
handleIncrease, | ||
handleReset, | ||
}; | ||
}; |
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.
useCount.ts로 파일명을 바꿔야할 것 같아요.
import type { Item } from '@/pages/MainPage/types'; | ||
|
||
export const sortItems = (items: Item[], order: 'ASC' | 'DESC'): Item[] => { | ||
return [...items].sort((a, b) => | ||
order === 'ASC' ? a.amount - b.amount : b.amount - a.amount | ||
); | ||
}; |
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.
MainPage 폴더 안에 같은 뎁스의 파일이 너무 많아 보기가 어려운 것 같아요.
저라면
MainPage/index.tsx
MainPage/utils/sortItems.ts
Mainpage/hooks/useCount.ts
MainPage/components/Theme.ts
이런 식으로 폴더링할 것 같아요.
<div> | ||
<Button onClick={handleLogout} theme="outline" size="responsive"> | ||
로그아웃 | ||
</Button> | ||
</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.
div으로 한 번 더 감쌀 필요는 없어보여요~
|
||
import { AuthContext } from '@/context/AuthContext'; | ||
|
||
export const useAuth = () => { |
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.
다른 커스텀 훅이랑 헷갈릴 것 같아서, useAuthContext 정도로 네이밍해봐도 좋을 것 같아요~
return ( | ||
<div> | ||
<h1>Login</h1> | ||
<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.
div로 감싼 이유가 있을까요?
return { | ||
id, | ||
setId, | ||
password, | ||
setPassword, | ||
}; |
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.
훅 내부에서 핸들링 하는 게 없고, 모든 걸 return한다면 훅으로 분리할 필요는 딱히 없어보여요~
const items = getItmes(); | ||
|
||
return ( | ||
<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.
div으로 감쌀 필요는 없어보여요~
미션1 머지로 인해 conflict가 발생중이어서, 해결부탁드립니다~ |
conflict 해결하여 머지하였습니다! @danmin20 |
안녕하세요, 강원대학교 허윤수입니다.
피드백을 위해 시간을 내주셔서 감사합니다!
현재
feat-heoyunsu
가 메인 브랜치이며, 과제를 단계별로 구현하였습니다. 아래는step1
PR에 대한 리뷰를 반영한step2
,step3
제출입니다.1단계 PR 링크
문제가 발생한 부분에 대해 도움을 구하고자 합니다.
themeKey
에 따라label
,title
,description
변환 및backgroundColor
변경 로직이 작동하지 않는데, 어느 부분에서 문제가 발생하는지 파악하지 못하겠습니다. 조언을 부탁드립니다.감사합니다!