-
Notifications
You must be signed in to change notification settings - Fork 26
[황휘태] sprint6 #105
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 #105
The head ref may contain hidden characters: "React-\uD669\uD718\uD0DC-sprint6"
Conversation
|
스프리트 미션 하시느라 수고 많으셨어요. |
styled component를 사용해서 스타일링을 했는데 한 컴포넌트의 코드가 너무 길어지는 느낌입니다. 스타일은 따로 빼놓는게 나을까요?넵넵 ! 코드가 길어지면 |
| */ | ||
| const onClickOpen = () => { | ||
| setIsOpen(!isOpen); | ||
| setIsOpen((prevState) => !prevState); |
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.
굿굿 ! updater function으로 사용하셨군요 ! 👍👍
| /** | ||
| * 업로드한 이미지의 정보를 URL 문자열로 변환 | ||
| * @param {event} e | ||
| */ | ||
| const onChangeUploadImg = (e) => { | ||
| if (imgPreviews !== "") { | ||
| return; | ||
| } | ||
|
|
||
| const previewImg = e.target.files[0]; | ||
| const imgLink = URL.createObjectURL(previewImg); | ||
| setImgPreviews(imgLink); | ||
| }; |
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.
주석을 꼼꼼히 작성하시는 모습 너무 좋습니다 👍
심지어 jsdoc으로 작성하셨네요 !
주석을 꼼꼼히 작성하시는건 정말 좋은 습관입니다 !
다만, 코드만 읽어도 충분한 코드(현재 onChangeUploadImg와 같이)는 onChangeUploadImg이라는 함수명으로 어떤 목적의 함수인지 명확히 알 수 있으며 복잡한 로직이 없으므로 생략해도 괜찮을거라 사료됩니다 !
그리고 jsdoc은 특히 재사용되는 "함수(또는 컴포넌트)"에 설명서로 정말 좋은 주석 방법입니다 👍👍
| const inputRef = useRef(); | ||
|
|
||
| /** | ||
| * 업로드한 이미지의 정보를 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.
설명서만 읽어보면 문자열로 변환만 해주는 것으로 보이네요 😉
| * 업로드한 이미지의 정보를 URL 문자열로 변환 | |
| * 업로드한 이미지의 정보를 URL 문자열로 변환하며 `ImgPreviews`를 갱신하는 이벤트 핸들러. |
| background-repeat: no-repeat; | ||
| background-position: center; | ||
| position: absolute; | ||
| z-index: 8888; |
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.
z-index도 상수로 관리해볼 수 있어요.
/* styles/variables.css */
:root {
--z-dropdown: 1000;
--z-modal: 1100;
--z-floating-button: 8888;
}
위와 같이 설정하시고 다음과 같이:
export const DeleteButton = styled.button`
...
z-index: var(--z-floating-button);
`;위와 같이 사용하시면 재사용성과 유지보수성을 높일 수 있습니다 😉
| /** | ||
| * 입력받은 가격을 원화 형식으로 변환하는 함수 | ||
| * @param {React.ChangeEvent<HTMLInputElement>} e | ||
| */ | ||
| const onChangePrice = (e) => { | ||
| let rawPrice = e.target.value.replace(/[^0-9]/g, ""); | ||
|
|
||
| if (rawPrice === "") { | ||
| e.target.value = ""; | ||
| return; | ||
| } | ||
|
|
||
| let formattedPrice = Number(rawPrice).toLocaleString("ko-KR"); | ||
| e.target.value = formattedPrice; | ||
| }; | ||
|
|
||
| /** | ||
| * Enter키를 입력으로 태그를 form에 등록하고 태그 입력 창을 비우는 만드는 함수 | ||
| * @param {KeyboardEvent} e | ||
| */ | ||
| const onKeyDownAppendTag = (e) => { | ||
| if (e.nativeEvent.isComposing) return; | ||
|
|
||
| if (e.key === "Enter") { | ||
| e.preventDefault(); | ||
|
|
||
| if (e.target.value.trim() !== "") { | ||
| append({ value: e.target.value }); | ||
| e.target.value = ""; | ||
| } | ||
| } | ||
| }; |
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.
깔끔한 핸들러입니다 👍👍
핸들러들이 깔끔하게 정의되었네요.
각 함수도 조건에 부합하지 않으면 조기에 반환하여 가독성을 높이는 패턴도 훌륭합니다 👍
| export const palette = { | ||
| gray900: "#111827", | ||
| gray800: "#1f2937", | ||
| gray700: "#374151", | ||
| gray600: "#4b5563", | ||
| gray500: "#6b7280", | ||
| gray400: "#9ca3af", | ||
| gray200: "#e5e7eb", | ||
| gray100: "#f3f4f6", | ||
| gray50: "#f9fafb", | ||
| blue: "#3692ff", | ||
| menu: "#4b5563", | ||
| }; |
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.
아까 z-index에 대한 피드백을 했었는데, 이렇게 palette처럼 z-index도 변수로 관리하셔도 되겠네요 😉
|
수고하셨습니다 휘태님 ~! 꾸준히 미션을 제출하시고 성장하시는 휘태님. 기초 프로젝트 땐 어떤 모습을 보여주실지 기대가 되네요 😉😉 |
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게
스타일은 따로 빼놓는게 나을까요?