-
Notifications
You must be signed in to change notification settings - Fork 35
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
[손재헌] Sprint5 #143
The head ref may contain hidden characters: "React-\uC190\uC7AC\uD5CC-sprint5"
[손재헌] Sprint5 #143
Conversation
…ithub-actions [Fix] delete merged branch github action
|
||
function App() { | ||
return ( | ||
<div className="App"> |
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.
root
가 컨텐츠를 감싸고 있는 역할을 하니까 여기 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.
페이지의 이동은 a
태그를 써주는게 더 자연스럽습니다.
<div className="nav"> | ||
<NavTab text="자유게시판" active="deactive" /> | ||
<NavTab text="중고마켓" active="active" /> | ||
</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.
순서의 의미가 없는 요소들을 나열하는 경우 ul
태그를 고려해보세요
} | ||
|
||
function BestSection() { | ||
const [items, setItems] = useState([]) |
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.
useState
에 디폴트 상태 []
좋습니다 !
데이터가 없을 경우 빈배열을 사용한다는 가정이 있어서 예상치 못한 동작을 할 확률이 줄어듭니다 !
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.
api 관련 함수를 다른 페이지에 정의 해주신 것 잘 하셨어요 !
앞으로 많은 api가 생길텐데 이렇게 함수로 따로 관리해주면 api 쪽 변경이 생길때 코드의 변경을 최소화 할 수 있어요.
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.
try-catch
문을 통해 네트워크 통신 에러가 발생했을때 핸들링 할 수 있는 코드를 작성해주시면 좋을 것 같아요.
async function getProducts({ page = 1, pageSize = 10, orderBy = '', keyword = '' }) { | ||
|
||
const query = `?page=${page}&pageSize=${pageSize}&orderBy=${orderBy}&keyword=${keyword}` | ||
const response = await fetch(`https://panda-market-api.vercel.app/products` + query); |
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.
https://panda-market-api.vercel.app
은 base url
이라고 합니다.
api 주소의 기본이 되는 부분인데, base url은 잘 바뀌지 않고 여러 api가 사용하기 때문에 전역변수 또는 환경변수로 관리해주시면 됩니다.
@@ -0,0 +1,11 @@ | |||
async function getProducts({ page = 1, pageSize = 10, orderBy = '', keyword = '' }) { |
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.
디폴트 파라미터를 잘 사용하셨어요 !
|
||
const [items, setItems] = useState([]); | ||
const [page, setPage] = useState(1); | ||
const [pageSize, setPageSize] = useState(10); |
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.
setPageSizes
는 사용되지 않고 있어요. 이런 상태는 일반 변수로 선언을 해주시면 됩니다.
const [pageSize, setPageSize] = useState(10); | |
const pageSize = 10 |
|
||
function ItemsSectionHeader({ orderBy, orderSetter, keywordSetter }) { | ||
|
||
const dropdown = document.querySelector(".orderBy-dropdown"); |
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.
리액트에서 DOM에 직접적인 접근을 할 때는 useRef
라는 것을 사용합니다.
https://ko.react.dev/reference/react/useRef
반응형은 어떻게 해야할지 전혀 모르겠어서 구현하지 못했습니다... 찾아보니 styled component 활용한다는 것 같던데 다른 방법도 있나요? 스프린트 미션 1~4 하셨을때 처럼 css와 미디어 쿼리를 이용하셔도 반응형을 구현 할 수 있습니다. |
build 결과물은 원래 git에 커밋이 안되나요??
|
요구사항
기본
심화
주요 변경사항
멘토에게