-
Notifications
You must be signed in to change notification settings - Fork 26
[ 김현화 ] sprint6 #120
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
[ 김현화 ] sprint6 #120
The head ref may contain hidden characters: "React-\uAE40\uD604\uD654-sprint6"
Conversation
feat: header.jsx 네비게이션 경로 수정/ css 수정 : active 모션 수정,
페이지 이동시 가로 사이즈 변동이 있어 레이아웃 width 100vw 로 수정
feat: 섹션 제목 css itemsList->common 으로 이동
feat: 미션 6완료
| import axios from "axios"; | ||
| const BASE_URL=import.meta.env.VITE_PRODUCT_BASE_URL; | ||
| const instance=axios.create({ | ||
| baseURL: BASE_URL, | ||
| }); |
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를 사용하셨군요 ! 👍
또한 Base URL을 환경 변수로 설정하셨네요 ! 훌륭합니다.
이제 현화님의 프로덕트는 환경에 유연하게 대처할 수 있는 프로덕트로 거듭났네요 👍👍
| return new URLSearchParams({ | ||
| page: page.toString(), | ||
| pageSize: pageSize.toString(), | ||
| orderBy: 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.
URLSearchParams를 사용하셨군요 ! 👍👍
훌륭합니다 ! 아무래도 문자열은 휴먼에러가 많이 발생할 수 밖에 없으므로 이렇게 처리해두시면 휴먼 에러를 줄이는 데에 분명 도움이 될거예요 😉
| //베스트 상품 | ||
| const BASE_URL = "https://panda-market-api.vercel.app/products?page="; | ||
| export async function getProductBest({ pageSize = 4 }) { | ||
| const BASE_URL = import.meta.env.VITE_PRODUCT_BASE_URL; |
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 함수 파일들은 BASE_URL을 사용할 필요가 없을 것으로 보입니다 😉
BASE_URL을 신경 쓸 곳은 instance 파일만 사용하면 되므로 파일들도 깔끔해지겠네요 👍
| const BASE_URL = import.meta.env.VITE_PRODUCT_BASE_URL; |
| const Button = ({ children, className, isDisabled = false, type, onClick }) => ( | ||
| <button | ||
| className={className} | ||
| disabled={isDisabled} | ||
| type={type} | ||
| onClick={onClick} | ||
| > | ||
| {children} | ||
| </button> | ||
| ); |
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.
이제 오호 버튼 공통 컴포넌트를 만드셨군요 👍
훌륭합니다 ! 다만, 현재는 버튼 공통 컴포넌트가 HTML의 <button>과 다를게 없어보이는군요!
현화님께서는 아마도, 버튼이란 것은 여러 페이지와 컴포넌트에 공통적으로 사용될 것이므로 해당 컴포넌트를 따로 정의하신 것으로 보입니다.
그렇다면 핵심은 "공통적인 스타일링을 하면서 커스터마이징에 유용한 컴포넌트"를 설계하시고자 하실 것으로 보여요.
지금은 확인 해보니, 스타일을 컴포넌트 호출부에서 주입해주고 있군요 !
조금 더 공통 컴포넌트로 다듬어 볼 수 있을 것 같아요.
예를 들어서 error, success, primary와 같은 컬러가 있을 수 있으나 현재 판다마켓 피그마에서는 primary 컬러만 존재하며, 사이즈에 따른 버튼 크기가 정의 되어있군요.
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.
어떻게 하면 될까?
다음은 GPT로 산출한 임의의 버튼 컴포넌트 예시입니다:
import React from "react";
import "./Button.css"; // CSS 파일로 스타일 분리
function Button({ children, size = "md", color = "primary", ...rest }) {
const classNames = `btn btn-${size} btn-${color}`;
return (
<button className={classNames} {...rest}>
{children}
</button>
);
}
export default Button;/* 공통 스타일 */
.btn {
border: none;
border-radius: 8px;
font-weight: 600;
cursor: pointer;
transition: background-color 0.2s ease;
}
/* 사이즈 */
.btn-sm {
font-size: 14px;
padding: 6px 12px;
}
.btn-md {
font-size: 16px;
padding: 8px 16px;
}
.btn-lg {
font-size: 18px;
padding: 12px 20px;
}
/* 색상 */
.btn-primary {
background-color: #007bff;
color: white;
}
.btn-primary:hover {
background-color: #0069d9;
}
.btn-success {
background-color: #28a745;
color: white;
}
.btn-success:hover {
background-color: #218838;
}
.btn-error {
background-color: #dc3545;
color: white;
}
.btn-error:hover {
background-color: #c82333;
}
/* 비활성화 */
.btn:disabled {
background-color: #ccc;
cursor: not-allowed;
}GPT는 컬러까지 정의해두었지만, 현재 요구사항에서는 컬러까진 필요 없더라도 사이즈는 참고해볼 수 있을 것 같네요 😉
또한, 현화님은 SCSS를 사용하고 계시므로 좀 더 CSS를 간결하게 처리할 수 있지 않을까 기대해봅니다 !
| //페이지네이션 최대 생성 갯수 | ||
| const LIST_MAX = 5; | ||
| const paginationArr = (startPage, maxPages) => { | ||
| const pages = []; | ||
| for (let i = 0; i < maxPages; i++) { | ||
| pages.push(startPage + i); | ||
| } | ||
| return pages; | ||
| }; |
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 handleKeyDown = (e) => { | ||
| if (e.key === "Enter") { | ||
| const value = e.target.value; | ||
| if (value !== "") { | ||
| onKeyDown(value); | ||
| e.target.value = ""; | ||
| } else { | ||
| return; | ||
| } | ||
| } | ||
| }; |
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.
else ... return 부분은 필요가 없을 것으로 보이는군요 !
| const handleKeyDown = (e) => { | |
| if (e.key === "Enter") { | |
| const value = e.target.value; | |
| if (value !== "") { | |
| onKeyDown(value); | |
| e.target.value = ""; | |
| } else { | |
| return; | |
| } | |
| } | |
| }; | |
| const handleKeyDown = (e) => { | |
| if (e.key === "Enter") { | |
| const value = e.target.value; | |
| if (value !== "") { | |
| onKeyDown(value); | |
| e.target.value = ""; | |
| } | |
| } | |
| }; |
해당 코드가 없어도 논리적으로 동일하게 동작하며 void 반환타입은 같으므로 필요 없을 것으로 보입니다 😉
| import selfDelete from "../assets/images/selfDelete.svg"; | ||
|
|
||
| //인풋 묶음 | ||
| function InputGroup({ |
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.
해당 컴포넌트는 Group 보다는 단일 Input으로 보이는군요 !
타입 별로 특정 컴포넌트를 레이아웃을 감싸고 <h3>를 추가하여 반환해주는 컴포넌트군요 !
Group이라는 키워드가 있으니까 2 개 이상의 인풋으로 구성될 것으로 오해될 소지가 있어보여요 ! 😉
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.
또한, 해당 컴포넌트의 역할이 단순히 <h3>을 추가해주는 정도라면 ?
InputGroup를 없애고 필요한 곳에 TextArea, ImageFileGroup, TextInput를 그대로 사용하는 것도 방법이겠어요.
예를 들어서:
<InputGroup
title={"상품이미지"}
id={"productImage"}
type={"file"}
{...commonProps}
/>현재 위와 같이 사용했다면 다음과 같이 사용해볼 수 있겠네요:
<div className={formStyle.inputGroup}>
<h3 className={formStyle.inputTitle}>"상품이미지"</h3>
<TextArea
id={"productImage"}
placeholder={"상품 이미지입니다."}
onChange={handleInputChange}
formStyle={formStyle}
/>근데 이렇게 되면 반복되는 div와 h3가 거슬릴 수도 있겠어요.
그럴 땐 다음과 같이:
<InputLabel className={className} label="상품이미지">
<TextArea
id={"productImage"}
placeholder={"상품 이미지입니다."}
onChange={handleInputChange}
formStyle={formStyle}
/>
</InputLabel>InputLabel라는 컴포넌트를 만들어서 공통적으로 처리해보는 방법도 고려해볼 수 있을 것 같군요 😊
이러한 피드백을 드리는 배경에는 input의 종류가 추가될 수록 inputGroup.jsx를 지속적으로 관리해야 하는 유지보수성 저하가 염려되어 조심스레 제안드려봅니다..! 😉
|
|
||
| return ( | ||
| <div className={formStyle.inputGroup}> | ||
| <h3 className={formStyle.inputTitle}>{title}</h3> |
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.
<label>을 활용해보시는 건 어떨까요?
| <h3 className={formStyle.inputTitle}>{title}</h3> | |
| <label className={formStyle.inputTitle} htmlFor={id}>{title}</label> |
라벨은 다음과 같은 이점이 있습니다:
<label> 을 <input> (en-US) 요소와 연결하면 몇 가지 이점이 있습니다:
- label 텍스트는 텍스트 입력과 시각적으로 관련이 있을뿐만 아니라 프로그래밍적으로도 관련이 있습니다. 예를 들어, 화면리더기(screenreader) 는 폼 입력(form input)에서 label 을 읽어서 보조기술(assistive technology) 사용자가 입력해야하는 텍스트가 무엇인지 더 쉽게 이해할 수 있게 합니다.
- 관련 label 을 클릭해서 input 자체에 초점을 맞추거나 활성화를 시킬 수 있습니다. (활성되어서)늘어난 누를 수 있는 영역(hit area)은 터치스크린 사용자를 포함해 입력하려하는 모든 사람에게 이점을 줍니다.
출처: MDN
|
수고하셨습니다 현화님 ~! 이번 미션도 정말 수고 많으셨습니다 현화님 👍👍 |
요구사항
배포사이트 :https://comfy-daifuku-7101e3.netlify.app/Additem
기본
심화
주요 변경사항
스크린샷
멘토에게