MISSON2 / 이규민 #10
MISSON2 / 이규민 #10
Conversation
Description: fontSize, backgroundColor, color, borderColor를 미리 설정합니다
Description: 카드 컴포넌트를 생성합니다
Description: theme에서 중복되는 색상들을 삭제하고 하나로 통일하였습니다
Description: 가장 기본적인 형태의 Card Component를 구현하였습니다(최소기능)
Description: module.css로 변경함에 따라 내부 className을 수정하였습니다
Description: 스탬프 구현을 위해 image도 assets 폴더에 추가하였습니다
Description: 쿠폰 구현을 위한 Coupon 이미지 assets에 추가
Description: Modal 별로 DefaultButton의 buttonText를 props를 통해 전달합니다
Description: 현재 적립하기의 경우 쿠폰 페이지 클릭시 stamp가 사라지고, 상태관리 또한 부적절하게 발생하고 있으므로, 리팩터링이 필요
Description: 기존 stampModal에서 stamp 적립시, 탭이 바뀌면 stamp들이 사라지는 문제를 해결
Descripiton: state 끌어올리기를 통해 스탬프를 적립하고, 스탬프 적립에 따라 쿠폰이 발급되며, 쿠폰이 사용되는 로직을 구현하였습니다
Description: stampNum의 조건을 수정해 스탬프가 10장이 되자마자 쿠폰을 발급하도록 하였습니다
개인적으로 나름의 명확한 기준을 세워 컴포넌트를 분리했다는 점에서 이 부분에 대한 이견은 없습니다! 개인적으로 네이밍에 대해 언급하고 싶은 2가지는 아래와 같은데요
|
저도 아직까지 여기에 대한 명확한 기준은 가지고있지 않은데요, 사실 정답이 없는게 당연하다고 생각합니다.
이정도 되는 것 같아요.
질문 주신 부분 중에서 hook과 util 분리를 고민중이신것 같았는데요, hook을 예시로 대강 작성해보자면 아래처럼 할 수 있을것 같습니다.(참고만 해주세요!) import { useState } from "react";
import { useToast } from './useToast'; // 토스트 노출 기능을 Hook으로 분리했다고 가정
const useCoupon = () => {
const [couponNum, setCouponNum] = useState(0);
const [showToast] = useToast();
const publishCoupon = (onSuccess) => {
setCouponNum((prev) => prev + 1);
// 이렇게 작성하거나
if (onSuccess && typeof onSuccess === 'function') {
// 쿠폰이 발급되었습니다 토스트 메시지를 직접 띄우든, 문자열만 리턴하든...
onSuccess();
}
// 이렇게 성공시 항상 토스트메시지를 보여주도록 할 수 있다
showToast('쿠폰 발급에 성공하였습니다!');
}
const consumeCoupon = () => setCouponNum((prev) => {
if (prev <= 0) {
showToast('사용가능한 쿠폰이 없습니다!');
return prev;
}
return prev - 1;
});
return {
couponNum,
publishCoupon,
consumeCoupon,
}
}
export default useCoupon;이렇게 작성했을때의 장점입니다.
여유가 되신다면 hook을 사용해서 리팩토링해보시면 좋을것같아요! |
jest로도 로직과 ui를 한번에 테스트하도록 할 수 있습니다. describe('show props', () => {
test('show가 true일 때, 달력이 노출된다.', () => {
// given
const datepicker = <Datepicker show={true} />;
// when
const { container } = render(datepicker);
// then
const calendar = container.querySelector(`.${DATEPICKER_CLASSNAMES.LAYER}`);
expect(calendar).toHaveClass(DATEPICKER_CLASSNAMES.LAYER_ACTIVE); // 캘린더가 특정 스타일로 렌더링되었는지를 className 포함 여부를 통해 확인할 수 있다.
});조금 더 긴 동작을 테스트해보고싶다면 cypress와 같은 툴을 사용해서 스토리북에서 잘 동작하는지 e2e테스트를 작성해볼 수도 있습니다. describe('DATETIME/Datepicker', () => {
before(() => {
cy.visitStorybook(); // cy는 cypress
});
it('Datepicker-Date 타입 > 날짜를 선택했을 때 해당 날짜가 선택되는지 확인합니다.', () => {
cy.loadStory('datetime datepicker', 'date 타입')
.find('.tbl-calendar__day')
.should('have.length', 35)
.each(($el) => {
const [day] = $el;
const clickable = ![...day.classList].some(
(className) => className === 'tbl-calendar__day--empty' || className === 'tbl-calendar__disabled'
);
if (clickable) {
const date = parseInt(day.outerText) < 10 ? `0${day.outerText}` : day.outerText;
cy.wrap($el)
.click()
.get('.input-tf')
// 클릭가능한 날짜를 선택했을 때, 예상한 날짜 값이 input에 들어가는지 확인합니다.
.should('have.value', '2019.11.' + date)
.get('.btn-calendar')
// 확인 후에는 캘린더를 확장합니다.
.click();
}
});
});
}); |
리액트 공식문서(https://react.dev/learn/state-as-a-snapshot#rendering-takes-a-snapshot-in-time)를 참고하시면 이해가 되실것같아요!
즉 리액트는 스냅샷처럼 렌더당시의 state값을 가지고 로직을 수행합니다. 처음 화면이 렌더링 될 때
그리고 추가적으로
요구사항 또는 기획을 코드로 그대로 옮길 수 있게 됩니다. const publishCoupon = () => {
setCouponNum(prev => prev+ 1);
}
const publishStamp = () => {
setStampNum(prev => prev + 1);
}
const resetStamp = () => {
setStampNum(0);
}
const handleSaveStamp = () => {
const MAX_STAMP_NUM = 10;
const nextStampNum = stampNum + 1;
if (nextStampNum >= MAX_STAMP_NUM) {
publishCoupon();
setToast(true);
setMessage('쿠폰이 발급되었습니다!');
resetStamp();
return;
}
publishStamp();
}; |
| import Coupon from '../atom/Coupon'; | ||
|
|
||
| const CouponModal = ({ buttonClick, couponNum }) => { | ||
| const useCoupon = () => { |
There was a problem hiding this comment.
추후에 hook으로 분리할거라면 이렇게 'use'가 붙는 네이밍은 훅과 혼동할 여지가 있을것같습니다. applyCoupon 등 다른 동사를 활용해보면 어떨까요?
|
|
||
| const renderCoupon = () => { | ||
| const coupon = []; | ||
| for (let i = 0; i < couponNum; i++) { |
There was a problem hiding this comment.
지금은 별도로 복잡한 요구사항이 없어 coupon, stamp 모두 카운팅하는 방식으로 작성했지만
스탬프별로, 쿠폰별로 다른 이름, 타이틀, 발급 날짜 등..을 갖게 되어 구별할 필요가 있어진다면 id필드를 가진 객체의 배열로 관리하는게 더 좋아보여요!
e.g.)
const coupon = [
{
id: 0,
title: '생일 할인 쿠폰',
publishDate: '2024-04-01',
expireDate: '2024-04-30',
},
{
id: 1,
title: '스탬프 10장 할인',
publishDate: '2024-03-29',
expireDate: '2024-12-31',
}
];
| github: 'https://github.com/Klomachenko', | ||
| youtube: 'https://www.youtube.com/channel/UCpxKdR3scZUtwjz_E-uIDyw', | ||
| }; | ||
| const linkClick = (link) => { |
There was a problem hiding this comment.
함수명은 습관적으로 동사로 시작하도록 지으면 좋습니당 e.g. clickLink
| for (let i = 0; i < stampNum; i++) { | ||
| stamp.push(<Stamp />); | ||
| } |
There was a problem hiding this comment.
배열의 내장메소드 map을 이용하면 다음과 같이 수정할 수 있습니다
const renderStamp = () => new Array(stampNum).map((i) => <Stamp key={i}/>);for문은 index의 증감을 개발자가 임의로 컨트롤할 수 있기 때문에, 로직이 복잡해질 경우 잠재적 오류를 발생시킬 여지가 있습니다. 특정한 상황(e.g. break문을 사용해야 하는 경우)을 제외하고는 배열의 순회를 프로그래밍언어에게 맡기도록 하는 map, forEach 등을 사용하는 것을 권장합니다.
There was a problem hiding this comment.
그리고 제가 알기로는 renderStamp처럼 함수로 정의하면 StampModal이 리렌더될때마다 renderStamp함수도 재실행되어 성능에 악영향을 줄 것 같은데요, 다음과 같이 변수로 값을 계산하여 저장해두면 리렌더시 재계산을 하지 않아 이렇게 작성하면 더 좋을것같습니다.
const stamps = new Array(stampNum).map((i) => <Stamp key={i}/>);
return (
<div>
<div className={styles.container}>
<div className={styles.buttonBox}>
<DefaultButton buttonText={'적립하기'} onClick={saveStamp} />
</div>
<div className={styles.contentBox}>{stamps}</div>
</div>
</div>
);
| const handleSaveStamp = () => { | ||
| setStampNum(stampNum + 1); | ||
| console.log(stampNum); | ||
| if (stampNum === 9) { |
There was a problem hiding this comment.
9와 같은 매직넘버는 반드시 변수, 상수화시키기를 권장합니다~
|
안녕하세요 규민님, 코드리뷰도 늦은 주제에 말이 많아서 놀라셨죠? |
1. 구현 모습
2. 해결 과정
배포 링크
기능 요구사항
TEST
플로우
파일 구조
파일 구조 보기
컴포넌트 분리의 기준
저번 1주차 미션 중 동완님의 미션에 기표님의 리뷰를 참고하였습니다.
결국, 아토믹 디자인 패턴도, 많은 사람들이 ‘쓰다 보니, 이렇게 하면 좋더라’라고 생각하였고,
이번 미션에 알맞게 컴포넌트를 세 단계로 분리하였습니다
폴더 구조(img)
Public vs assets
현재 쿠폰이나, 스탬프 등의 이미지는, ‘디자인’을 나타내는 이미지이며,
사용자에게 쿠폰이나 스탬프의 시각적인 정보를 전달합니다.
또한, 해당 이미지를 사용하는 컴포넌트가 사용자에 의해 ‘동적’으로 사용되므로, assets 폴더에 넣는 것으로 결정하였습니다.
스타일
아쉬운 점
해결 방법
상태 관리
현재 상단에 작성해놓은 플로우에 따르면 ‘쿠폰’과 ‘스탬프’가 유기적으로 상호작용 하는 것을 볼 수 있습니다.
하지만, 컴포넌트 분리를 생각해 보았을 때, 쿠폰과 스탬프는 현재 부모, 자식 요소가 아닙니다.
위와 같은 상황에서 2가지 선택지가 있다고 생각합니다.
상태 끌어올리기
위에서 말했듯이, 현재 ‘CouponModal’과 ‘StampModal’이 서로 반드시 데이터를 공유해야 합니다.
하지만, 현재 CouponModal과 StampModal 간에는 props 전달 방식으로 데이터 공유가 불가능합니다.
이러한 경우 해결 방법은
이렇게 하는 방법을 “state 끌어올리기”라고 부릅니다.
(너무 복잡하게 실행되는 느낌입니다. 그냥 handleSaveStamp 자체를 넘기는 방법으로 리팩터링 필요)
(너무 복잡하게 실행되는 느낌입니다. 그냥 handleUseCoupon 자체를 넘기는 방법으로 리팩터링 필요)
상태 관련 질문점
어디까지 공통의 부모 컴포넌트에서 관리해야 하는가?
3. To 리뷰어에게
또한, TEST 목록이 적절한지도 궁금합니다.
아래는 리팩터링 할 목록입니다. 혹시 올바르게 찾았는지, 더 할 게 있는지 확인해주시면 매우 감사하겠습니다.
스탬프가 10이 아니라 11번째 클릭때 발급된다는 메시지
분명 state를 변경하도록 했는데 클릭시에 1이 아니라 0이 콘솔에 출력되는 이유가 궁금합니다.
쿠폰이 순차적으로 삭제만 될 뿐
만약 쿠폰의 종류가 달라 특정 쿠폰을 사용하게 된다면…?
특정 쿠폰을 클릭해서 삭제해야 하는 경우엔 리스트에 담는 형식으로 구현해야 할 것 같습니다!
리스트에 담아서, 클릭한 쿠폰의 인덱스를 삭제하는 방식!
(현재는 숫자로 관리해서 숫자만큼 반복하여 stamp, coupon을 보여주고 있습니다)
(숫자로 관리한 이유는 뭔가 테스트할 때 숫자가 조금 더 직관적일 것이라 생각했기 때문입니다.)
귀중한 시간 내주셔서 정말 감사합니다!