-
Notifications
You must be signed in to change notification settings - Fork 31
[김희성] Sprint5 #149
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 #149
The head ref may contain hidden characters: "React-\uAE40\uD76C\uC131"
Conversation
dongqui
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.
희성님! 프로젝트 이후에도 열심히 달려주시네요! 👍 👍
리엑트는 기본 사용은 역시 능숙하신 거 같습니다 :) 컴포넌트화, 커스텀 훅 작성도 신경 써주시면 더욱 좋을 거 같아요!
svg로 저장하는 이미지와 png로 저장하는 이미지의 차이가 어떤 게 있는지, next에서 적합한 건 svg라고 들었다는 팀원의 말을 얼핏 들었는데 잘 모르는 부분이어서 여쭤봅니다.
->
음.. 팀원분이 어떤 관점에서 next에서는 svg가 적합하다고 말씀하신 건지 잘 모르겠지만.. 일단 보통은 렌더링 방식 보다는 이미지 성격에 따라 선택합니다!
svg는 벡터 이미지, png는 비트맵 이미지 입니다! 즉, svg는 수학적인 계산을 통해 렌더링하게 되고 png는 고정된 픽셀 정보를 토대로 렌더링하게 됩니다. 따라서 화면이 커지거나 작아질 때 svg는 계산 후 렌더링 하므로 유연하게 대처 가능하지만, png는 픽셀이 깨지는 등의 문제가 발생하죠!
그 대신 복잡한 이미지의 경우 SVG는 많은 계산이 필요하게 됩니다. 그래서 보통 svg는 아이콘, 로고 등 단순한 이미지에 많이 사용하고 일반적인 이미지에는 png가 사용되는 경우가 많습니다 :)
서버에 잘못 post된 이미지들의 경우 No Image 이미지가 뜨도록 설정을 하였으나, 해당의 경우에서만 이미지 로딩이 느립니다. 그 이유가 무엇인지 궁금합니다.
-> 일단 이미지가 바로 받아지는 것이 아니라, 실패했을 때 이미지를 받기 시작하므로 보통 이미지 보다는 늦을 거 같네요! 그리고 무엇보다 해당 이미지가 굉장히 큽니다! (제가 봤던 페이지에서는 다른 이미지랑 30배이상 차이났네요..!)

웹 사이트 리소스 중 가장 큰 부분을 차지하는 것 중 하나가 이미지입니다. 그래서 이미지를 최적화 하시는 것도 굉장히 중요해요 :) next/image 같은 것들을 활용하는 것도 그 이유죠!
tablet에서 pc로 넘어갈 때 2개의 사진이 4개의 사진으로 바뀌지 않던데, 이 부분은 어떻게 해결하면 좋을까요? 모바일에서 window.innerWidth를 기준으로 상품의 개수를 다르게 하면, 새로고침을 해야만 반영됩니다. 새로고침을 하지 않고도 미디어쿼리의 반응형할 때 처럼 바로 반영되게 할 수 있나요?
전체 상품 부분의 header 파트에서는 pc나 tablet의 경우, 왼쪽과 오른쪽이 나뉘어져서 그 기준으로 구현을 진행했습니다. 하지만 mobile은 상품 등록하기 버튼이 상단으로 나와야해서 다시 로직을 짜야하는데 지금 제 코드 말고 더 좋은 접근 방식이 있는지 궁금합니다.
최신순과 좋아요순이 나타나는 드롭다운을 구현하였습니다. display: none을 사용하여서 하였는데 이 방법 말고 더 좋은 접근이 있을까요?
-> 리뷰 참고해 주세요~! :)
| {isOpen && ( | ||
| <ul className="dropdown-list"> | ||
| <li className="dropdown-item" onClick={() => handleSelect("recent")}> | ||
| 최신순 |
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.
깔끔하게 잘 컴포넌트화 해주셨습니다! 여기서 최신순, 좋아요순 같은 도메인 로직은 밖에서 정의한다면 컴포넌트 재사용을 높일 수 있습니다 :)
| setCurrentPage(pageNumber); | ||
| }; | ||
|
|
||
| useEffect(() => { |
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.
newPageList가 pageCount에 의해 결정된다면 굳이 상태 값으로 정의하지 않아도 됩니다! :)
const pageList = Array.from({ length: pageCount }, (_, i) => i + 1);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.
말씀하신 상태 값이라는 게 어떤 부분인지 잘 모르겠습니다
| } | ||
| }, [pageCount]); | ||
|
|
||
| useEffect(() => { |
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.
위와 마찬가지입니다! visiblePage는 상태 값으로 정의할 필요 없고, useEffect도 굳이 사용하지 않아도 됩니다 :)
const startIndex = currentGroup * groupSize;
const visiblePages = pageList.slice(startIndex, startIndex + groupSize);상태 값, useEffect는 불필요한 렌더링을 발생 시키고, 코드의 흐름 추적을 어렵게 하는 경향이 있어 남용하는 것을 조심하셔야 합니다~!
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.
제가 생각하는 상태의 정의랑 다른 것 같다는 생각이 들어요. 말씀하시는 상태라는 게 어떤 것인지 구체적으로 궁금합니다.
(위에 질문 주신 부분까지 같이 작성하겠습니다~! )
제가 말씀 드린 상태 값은 아래 정의하신 부분입니다!
const [pageList, setPageList] = useState([]);
const [visiblePages, setVisiblePages] = useState([]);위 값들은 상태 값으로 정의하실 필요 없습니다!
먼저 pageList를 보면, 아래 useEffect에서 set되고 있습니다.
// 해당 useEffect를 1번 useEffect로 부르겠습니다.
useEffect(() => {
if (pageCount > 0) {
const newPageList = Array.from({ length: pageCount }, (_, i) => i + 1);
setPageList(newPageList);
}
}, [pageCount]);pageCount가 변하면, useEffect callback을 실행시키고, 새로운 페이지를 setPageList에 넣어 상태를 업데이트 해주고 있습니다.
그런데 pageList가 pageCount에 의해 결정되는 값이라면, 굳이 상태 값으로 저장하고, useEffect를 사용할 필요가 없습니다.
pageCount 변경되면 거기에 맞게 바로 pageList를 구해주시면 됩니다.
const pageCount = Math.ceil(totalCount / itemsPerPage);
const pageList = Array.from({ length: pageCount }, (_, i) => i + 1);visiblePages도 마찬가지입니다. :)
아래 useEffect에서 set되고 있죠!
// 2번 useEffect
useEffect(() => {
// 현재 그룹에서 보여줄 페이지들
const startIndex = currentGroup * groupSize;
setVisiblePages(pageList.slice(startIndex, startIndex + groupSize));
}, [currentGroup, pageList]);currentGroup, pageList에 의해 결정되는 값이라면 상태 값으로 정의할 필요 없이 바로 구하시면 됩니다!
그렇게 불필요한 상태와 useEffect를 제거하면 아래 같이 작성할 수 있습니다.
const [currentGroup, setCurrentGroup] = useState(0);
const [currentPage, setCurrentPage] = useState(1);
const pageCount = Math.ceil(totalCount / itemsPerPage);
const groupSize = 5;
const pageList = Array.from({ length: pageCount }, (_, i) => i + 1);
const startIndex = currentGroup * groupSize;
const visiblePages = pageList.slice(startIndex, startIndex + groupSize);기존의 코드 흐름을 보면,
pageCount 변경 (totalCount 혹은 itemsPerPage 프랍스 값 변경) -> 렌더링 -> 1번 useEffect callback 실행 -> setPageList 실행 -> 렌더링 발생 -> pageList 변경 -> 2번 useEffect callback 실행 -> setVisiblePages -> 렌더링 발생
수정한 코드의 흐름을 보면,
pageCount 변경 (totalCount 혹은 itemsPerPage 프랍스 값 변경)-> 렌더링
이렇게 정리할 수 있겠습니다..!
상호작용 등에 의해 결정되는 값이 아니라 특정 값에 의해 결정되는 값이라면 굳이 상태 값으로 저장할 필요 없습니다~! :)
| const pageNumber = searchParams.get("page"); | ||
|
|
||
| const getResponsiveLimit = () => { | ||
| const width = window.innerWidth; |
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.
tablet에서 pc로 넘어갈 때 2개의 사진이 4개의 사진으로 바뀌지 않던데, 이 부분은 어떻게 해결하면 좋을까요? 모바일에서 window.innerWidth를 기준으로 상품의 개수를 다르게 하면, 새로고침을 해야만 반영됩니다. 새로고침을 하지 않고도 미디어쿼리의 반응형할 때 처럼 바로 반영되게 할 수 있나요?
-> 현재 window size가 변하더라도 리엑트가 알 수 있는 방법이 없습니다!
window resize 이벤트를 이용해 보세요 :)
useEffect(() => {
window.addEventListner('resize', () => {
//...
});
//..
}, [])| const [bestProducts, setBestProducts] = useState([]); | ||
|
|
||
| const [searchParams, setSearchParams] = useSearchParams(); | ||
| const pageNumber = searchParams.get("page"); |
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.
쿼리스트링을 활용해 주셨군요! 👍 여유가 되신다면 정렬, 검색도 쿼리스트링을 이용해 주시면 좋을 거 같아요 :)
| ))} | ||
| </article> | ||
| </section> | ||
| <section className="all-section"> |
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 파트에서는 pc나 tablet의 경우, 왼쪽과 오른쪽이 나뉘어져서 그 기준으로 구현을 진행했습니다. 하지만 mobile은 상품 등록하기 버튼이 상단으로 나와야해서 다시 로직을 짜야하는데 지금 제 코드 말고 더 좋은 접근 방식이 있는지 궁금합니다.
-> 더 좋은 접근 방식이라기 보다는.. css로 할 수 있기는 합니다!
all-header-right div를 제거하고,
화면 크기가 클 때는 .search-box에 margin-left: auto;
모바일은 .all-header에 flex-wrap: wrap; 그리고 각각 요소에 order로 순서 바꾸면 될 거 같네요 🤔
아니면 grid-template-areas써보셔도 좋습니다!
근데 지금 하신 방법도 괜찮습니다~! :)
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.
코드가 너무 길어져서 찝찝하시다면 컴포넌트화를 통해 jsx를 조금 더 추상화 해보시면 더욱 좋을 거 같아요!
BestProducts 관련된 상태와 jsx를 따로모으고, AllProducts 관련된 상태와 jsx를 따로 모으는 거죠! 그리고 AllProducts 내부에서 Header관련된 코드를 또 따로 모을 수도 있을 거 같습니다 :)
getProducts는 훅으로 만들어 재사용할 수 있을거구요!
꼭 UI나 재사용을 위해서만 컴포넌트를 만드는 것은 아닙니다. 관련된 코드를 모아 좀 더 가독성 좋게 만들고, 관련 코드를 찾기 쉽도록 컴포넌트화를 하기도 합니다!
|
|
||
| import "./login.css"; | ||
|
|
||
| export default function Login() { |
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.
부지런히 옮겨주셨군요 🤣 👍
| padding: 9px; | ||
| } | ||
|
|
||
| .dropdown-button-text { |
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.
최신순과 좋아요순이 나타나는 드롭다운을 구현하였습니다. display: none을 사용하여서 하였는데 이 방법 말고 더 좋은 접근이 있을까요?
-> 여기 말씀하시는 게 맞을까요~?
잘 하셨습니다 :)
이 방법이 아니면, useMediaQuery 같은 훅을 만들어서 모바일 크기 때 상태에 따라 조건부 렌더링을 하는 방법도 있습니다 :) 이건 더 좋은 접근이라기 보다는 그냥 다른 방법입니다 🤣 지금 충분히 잘 해주셨어요 👍
요구사항
기본
심화
주요 변경사항
스크린샷
PC버전
Tablet 버전
Mobile 버전
멘토에게