-
Notifications
You must be signed in to change notification settings - Fork 39
[김수민] Sprint6 #183
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 #183
The head ref may contain hidden characters: "React-\uAE40\uC218\uBBFC-sprint6"
Conversation
GANGYIKIM
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.
수민님 6번째 미션 제출 수고하셨습니다!
인풋과 폼 다루는게 쉽지 않은데 정말 잘 하셨어요.
다음 미션도 화이팅입니다!
| ); | ||
| } | ||
|
|
||
| export const useAddItemForm = () => useContext(AddItemFormContext); |
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.
💊 제안
해당 context를 사용하는 훅을 정의하셔서 사용성을 높으신 것 좋습니다.
다만 아래처럼 context 내부에서 해당 context에 접근하지 않을때의 에러도 추가해주시면 더 좋을 것 같습니다!
| export const useAddItemForm = () => useContext(AddItemFormContext); | |
| export const useAddItemForm = () => { | |
| const _context = useContext(AddItemFormContext); | |
| if (!_context) throw new Error(/** error msg */); | |
| return _context; | |
| } |
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.
💬 여담
contextAPI를 사용하실 때는 주의해야할 점이 있습니다.
context를 통해 값을 전달할때는 해당 값이 변경되면 구독하는 컴포넌트에서 리렌더링이 발생할 수 있다는 것을 염두에 두고 작업을 해야합니다. (아래 코드의 경우 객체를 value로 공유하고 있어, 렌더링시 새로운 객체가 생성되어 참조가 변경되므로 object.is로 단순 비교시 변경으로 판단되어 리렌더링을 유발합니다. )
이를 해결하실 수 있는 다양한 방법이 있으나 가장 중요한 것은 동작에 대해 이해하는 것이므로 아래의 글들을 읽어보시는 것을 추천드립니다. 추후에는 상태관리 라이브러리를 사용해서 이러한 기능을 구현하실 가능성이 큰데 그때도 context 동작에 대해 이해하고 계시는 것이 도움이 되실거에요!
https://ko.react.dev/reference/react/useContext#optimizing-re-renders-when-passing-objects-and-functions
https://velog.io/@velopert/react-context-tutorial
| name='' | ||
| id='' |
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이랑 input은 연결해주세요~
| if (newTag.includes(' ')) { | ||
| alert('태그에는 띄어쓰기가 포함될 수 없습니다.'); | ||
| return; | ||
| } | ||
| if (newTag && !tags.includes(newTag)) { |
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.
💊 제안
태그에 공백이 포함되면 alert로 유저에게 피드백을 주신 것처럼 중복의 경우도 안내를 해주시면 일관성과 UX 측면에서 더 좋을 것 같아요.
| import styles from '../styles/AddItemLists.module.css'; | ||
| import AddItemImage from './AddItemImage'; | ||
| import AddItemTag from './AddItemTag'; | ||
| import { useState, useRef } from 'react'; |
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.
❗️ 수정요청
| import { useState, useRef } from 'react'; | |
| import { useRef } from 'react'; |
| const handlePriceChange = (e) => { | ||
| const el = inputRef.current; | ||
| const rawValue = e.target.value; | ||
| const cursorPos = el.selectionStart; | ||
|
|
||
| // 쉼표 제거 | ||
| const cleanValue = rawValue.replace(/[^0-9]/g, ''); | ||
| const numericValue = cleanValue === '' ? 0 : Number(cleanValue); | ||
|
|
||
| // 상태 업데이트 | ||
| changePrice(numericValue, rawValue, cursorPos); | ||
| }; | ||
| const changePrice = (value, rawValue, prevCursorPos) => { | ||
| setPrice(value === 0 ? '' : value.toLocaleString()); |
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.
💊 제안
변수 el, cursorPos의 경우 changePrice 함수에 인수로 사용되기 위한 값이므로 아래처럼 작성하시는 것이
더 가독성 측면에서 좋을 것 같습니다~
| const handlePriceChange = (e) => { | |
| const el = inputRef.current; | |
| const rawValue = e.target.value; | |
| const cursorPos = el.selectionStart; | |
| // 쉼표 제거 | |
| const cleanValue = rawValue.replace(/[^0-9]/g, ''); | |
| const numericValue = cleanValue === '' ? 0 : Number(cleanValue); | |
| // 상태 업데이트 | |
| changePrice(numericValue, rawValue, cursorPos); | |
| }; | |
| const changePrice = (value, rawValue, prevCursorPos) => { | |
| setPrice(value === 0 ? '' : value.toLocaleString()); | |
| const handlePriceChange = (e) => { | |
| const rawValue = e.target.value; | |
| const cleanValue = rawValue.replace(/[^0-9]/g, ''); | |
| const numericValue = cleanValue === '' ? 0 : Number(cleanValue); | |
| changePrice(numericValue, rawValue); | |
| }; | |
| const changePrice = (value, rawValue) => { | |
| const prevCursorPos = inputRef.current.selectionStart; | |
| setPrice(value === 0 ? '' : value.toLocaleString()); |
| // 상태 업데이트 | ||
| changePrice(numericValue, rawValue, cursorPos); | ||
| }; | ||
| const changePrice = (value, rawValue, prevCursorPos) => { |
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.
💊 제안
value === 0 ? '' : value.toLocaleString() 부분이 반복되니 변수에 저장하시고 사용하시면 더 좋을 것 같아요.
const changePrice = (value, rawValue, prevCursorPos) => {
const formatted = value ? value.toLocaleString() : "";
setPrice(formatted);
setTimeout(() => {
...
// 새 포맷된 문자열 <= ❗️ 불필요
// const formatted = value === 0 ? '' : value.toLocaleString();
for (let i = 0; i < formatted.length; i++) { ... }
}
}또한 setTimeout에 콜백도 따로 이름을 붙여 분리해주시면 가독성에 더 좋을 것 같습니다~
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://dduminipandamarket.netlify.app/additem
요구사항
기본
상품 등록
심화
주요 변경사항
스크린샷
PC
M
멘토에게