-
Notifications
You must be signed in to change notification settings - Fork 26
[한장희] sprint5 #121
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 #121
The head ref may contain hidden characters: "React-\uD55C\uC7A5\uD76C-sprint5"
Conversation
|
스프리트 미션 하시느라 수고 많으셨어요. |
| <BrowserRouter> | ||
| <Routes> | ||
| <Route path="/" element={<Home />}></Route> | ||
| <Route path="/items" element={<ProductList />}></Route> | ||
| <Route path="*" element={<NotFound />}></Route> | ||
| </Routes> | ||
| </BrowserRouter> |
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.
굿굿 ! 정말 빠르게 react-router-dom을 도입하셨군요 !
역시 프로젝트 때 한 번 경험해보셔서 그런지 척척 잘 해내시네요 😊😊
|
|
||
| return ( | ||
| <div className="flex flex-col gap-3 "> | ||
| <h2 className="text-xl font-bold ">베스트 상품</h2> |
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.
현재 h1가 없이 바로 h2 태그로 되었군요 !
시맨틱을 고려한다면 h 태그는 순차적으로 기입하는 것을 권장드립니다 !
h1 다음은 h2. 그리고 h3...
이런식으로 순차적으로 기입할 것을 MDN에서 권장하고 있습니다. 😊
MDN: 제목 단계를 건너뛰는 것을 피하세요. 언제나
<h1>로 시작해서,<h2>, 순차적으로 기입하세요.
현재 마땅히 h1을 넣을 곳이 없다면 임시로 판다마켓 로고에 h1를 감싸고 alt에 문서의 제목("판다마켓")을 지정하는 방법도 있겠네요 ~! (물론 추 후 페이지 별로 적합한 h1으로 설정해두면 더욱 좋습니다 😉)
| import { useResponsivePage } from "../../hooks/useResponsivePage"; | ||
| import ProductGrid from "./../common/ProductGrid"; | ||
|
|
||
| const BestProduct = ({ products }) => { |
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.
Product 모음(복수)이기에 Products가 더 낫지 않을까 싶어서 제안드려봐요 ~!
| const BestProduct = ({ products }) => { | |
| const BestProducts = ({ products }) => { |
| const { setCurrentPage, getPageNumber, currentPage } = usePagination(); | ||
| const { products, totalProductCount } = useGetProducts({ | ||
| pageSize, | ||
| orderBy, | ||
| currentPage, | ||
| }); | ||
| const { normalPicSize, normalPicContainerSize } = useResponsivePage(); |
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.
크으 ~ 상태 관리를 목적에 따라 분리하기 위하여 커스텀 훅으로 설계하셨군요 !
미팅에서도 말씀드렸지만 이러한 도전들 너무 좋습니다. 여러 기법들을 활용하시는 장희님의 학습 태도를 보면 정말 빠르게 성장하실거라고 생각이 드네요 😊😊😊
| <ProductGrid | ||
| products={products} | ||
| gridSize={bestPicContainerSize} | ||
| picSize={bestPicSize} | ||
| /> |
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.
인기 상품, 최근 상품 둘 다 ProductGrid를 사용하고 있군요 !
재사용성 좋게 잘 만드셨어요 굿굿 👍
| const VALUES = { | ||
| recent: "최신순", | ||
| favorite: "좋아요순", | ||
| }; | ||
|
|
||
| const Dropdown = ({ handleSelect, orderBy }) => { | ||
| const [isDropdown, setIsDropdown] = useState(false); | ||
| const [dropdownValue, setDropdownValue] = useState(VALUES[orderBy]); |
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.
(제안) Dropdown이 현재 ProductListSection에서만 사용할 수 있도록 설계되었군요 !
아마 ProductListSection/ 내부에 있는 것으로 봐서 의도하신 것으로 보여요.
다만, 추 후 이러한 드롭다운을 다른 곳에서 사용되는 것도 염두해서 범용성 좋은 컴포넌트를 설계해보시는 것도 학습에 도움 되실 것 같아서 제안드려봅니다 😊
해당 컴포넌트에 VALUES가 상수로 정의되어 있어서 "최신순", "좋아요순"이라는 아이템만 가질 수 있도록 설계되어 있어요.
해당 상수를 부모가 가지게 하고 props로 전달줄 수 있을 것으로 보여요:
export const SORT_OPTIONS = [
{ value: "recent", label: "최신순" },
{ value: "favorite", label: "좋아요순" },
];
function Container() { // ...
// .... 그리고 렌더링 단에서 다음과 같이
<Dropdown
options={SORT_OPTIONS}
value={orderBy}
onChange={(val) => setOrderBy(val)}
/>그리고 현재 Dropdown의 dropdownValue는 Container의 orderBy와 용도가 중복되는 것으로 보이므로 open(현재 isDropdown) 상태만 가지도록 해도 될 것 같군요 !
export default function Dropdown({ options, value, onChange }) {
const [open, setOpen] = useState(false);
const selected = options.find((o) => o.value === value) ?? options[0];
const handleSelect = (val) => {
onChange(val);
setOpen(false);
};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 클래스들을 모아두셨군요 !
파일명에 오해가 있을 수 있을 것 같아서 ProductGridClasses.js는 어떨지 제안드려봅니다 😉
| export const useResponsivePage = () => { | ||
| const [pageSize, setPageSize] = useState(10); | ||
| const [bestPageSize, setBestPageSize] = useState(4); | ||
| const [isMobile, setIsMobile] = useState(false); | ||
| const [normalPicSize, setNormalPicSize] = useState("normalPicSize"); | ||
| const [normalPicContainerSize, setNormalPicContainerSize] = | ||
| useState("grid224"); | ||
| const [bestPicContainerSize, setBestPicContainerSize] = useState("grid228"); | ||
| const [bestPicSize, setBestPicSize] = useState("bestPicSize"); | ||
|
|
||
| useEffect(() => { | ||
| function updatePageSize() { | ||
| if (window.matchMedia("(min-width: 1200px)").matches) { | ||
| setPageSize(10); | ||
| setBestPageSize(4); | ||
| setIsMobile(false); | ||
| setNormalPicSize("normalPicSize"); | ||
| setNormalPicContainerSize("grid224"); | ||
| setBestPicContainerSize("grid228"); | ||
| setBestPicSize("bestPicSize"); | ||
| } else if (window.matchMedia("(min-width:680px)").matches) { | ||
| setPageSize(6); | ||
| setBestPageSize(2); | ||
| setIsMobile(false); | ||
| setNormalPicSize("normalPicSize"); | ||
| setNormalPicContainerSize("grid224"); | ||
| setBestPicContainerSize("grid343"); | ||
| setBestPicSize("bestPicTabletMobile"); | ||
| } else { | ||
| setPageSize(4); | ||
| setBestPageSize(1); | ||
| setIsMobile(true); | ||
| setNormalPicSize("normalPicMobile"); | ||
| setNormalPicContainerSize("grid168"); | ||
| setBestPicContainerSize("grid343"); | ||
| setBestPicSize("bestPicTabletMobile"); | ||
| } | ||
| } | ||
| updatePageSize(); | ||
| window.addEventListener("resize", updatePageSize); | ||
| return () => window.removeEventListener("resize", updatePageSize); | ||
| }, []); | ||
|
|
||
| return { | ||
| pageSize, | ||
| bestPageSize, | ||
| isMobile, | ||
| normalPicSize, | ||
| normalPicContainerSize, | ||
| bestPicSize, | ||
| bestPicContainerSize, | ||
| }; | ||
| }; |
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 CONFIG = {
desktop: {
pageSize: 10,
bestPageSize: 4,
isMobile: false,
normalPicSize: "normalPicSize",
normalPicContainerSize: "grid224",
bestPicContainerSize: "grid228",
bestPicSize: "bestPicSize",
},
tablet: {
pageSize: 6,
bestPageSize: 2,
isMobile: false,
normalPicSize: "normalPicSize",
normalPicContainerSize: "grid224",
bestPicContainerSize: "grid343",
bestPicSize: "bestPicTabletMobile",
},
mobile: {
pageSize: 4,
bestPageSize: 1,
isMobile: true,
normalPicSize: "normalPicMobile",
normalPicContainerSize: "grid168",
bestPicContainerSize: "grid343",
bestPicSize: "bestPicTabletMobile",
},
};
function getDevice() {
if (window.matchMedia("(min-width: 1200px)").matches) return "desktop";
if (window.matchMedia("(min-width: 680px)").matches) return "tablet";
return "mobile";
}
export const useResponsivePage = () => {
const [device, setDevice] = useState(getDevice);
useEffect(() => {
const onResize = () => setDevice(getDevice());
onResize();
window.addEventListener("resize", onResize);
return () => window.removeEventListener("resize", onResize);
}, []);
// 파생값은 상태에 의한 계산으로만 😊
const values = useMemo(() => CONFIG[device], [device]);
return values; // 결국 { pageSize, bestPageSize, isMobile, normalPicSize, ... }
};"어떤 값에 의해서 바뀌는가?"를 생각해보면 결국 matchMedia에 따른 device를 파악하고 나머지 값들은 특정 디바이스에 따른 고정된 값들을 제공하고 있어요. 😉
따라서 device 상태에 의존되는 나머지 값들은 상수 객체로 지정하여 사용해볼 수 있어요. 🚀
아마 그대로 적용하셔도 호환될 거라고 사료되긴 하나 혹시 안될 수도 있습니다..👀👀
| const API_BASE_URL = import.meta.env.VITE_BASE_URL; | ||
|
|
||
| const instance = axios.create({ | ||
| baseURL: API_BASE_URL, | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| }); |
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.
굿굿 ! 환경변수 지정과 인스턴스 생성 ! 훌륭합니다.
환경에 유연하고 base url을 기반으로 하여 유지보수에 용이하겠네요 ! 👍👍
| export default async function getProductLists( | ||
| page = 1, | ||
| pageSize = 10, | ||
| orderBy = "recent" | ||
| ) { | ||
| try { | ||
| const response = await instance.get( | ||
| `products?page=${page}&pageSize=${pageSize}&orderBy=${orderBy}` | ||
| ); | ||
| return response.data; | ||
| } catch (error) { | ||
| throw new Error(`상품을 불러오는데 실패하였습니다. ${error.message}`); | ||
| } | ||
| } |
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.
axios는 쿼리를 객체로 넘길 수 있습니다 😉
| export default async function getProductLists( | |
| page = 1, | |
| pageSize = 10, | |
| orderBy = "recent" | |
| ) { | |
| try { | |
| const response = await instance.get( | |
| `products?page=${page}&pageSize=${pageSize}&orderBy=${orderBy}` | |
| ); | |
| return response.data; | |
| } catch (error) { | |
| throw new Error(`상품을 불러오는데 실패하였습니다. ${error.message}`); | |
| } | |
| } | |
| export default async function getProductLists( | |
| page = 1, | |
| pageSize = 10, | |
| orderBy = "recent" | |
| ) { | |
| try { | |
| const response = await instance.get("products", { | |
| params: { page, pageSize, orderBy }, | |
| }); | |
| return response.data; | |
| } catch (error) { | |
| throw new Error(`상품을 불러오는데 실패하였습니다. ${error.message}`); | |
| } | |
| } |
번거로운 문자열 말고 객체 한 입 하시죠 😋😋
|
미션 수행하시느라 수고하셨습니다 장희님 ~~~ 수고 정말 많으셨으며 언제나 화이팅입니다 💪💪 |
요구사항
기본
중고마켓 반응형
심화
주요 변경사항
스크린샷
멘토에게