[ Fix ] 모달 내 버그 해결 및 리팩토링

[seueooo]

🔥 Related Issues

• close [ Fix ] 모달 내 버그 해결 #191

✅ 작업 리스트

• 중복 url에 대한 처리
• 에러메세지 표시 후, 에러메세지와 input창 초기화 안 되던 문제 해결
• ButtonCategoryCommon 리팩토링
• CategoryModalRight에서 확인/취소 버튼을 눌렀을 때, CategoryModalLeft 초기화되도록 해결
• modal left, right 컴포넌트 병합 및 TitleMoribSet 컴포넌트 생성

🔧 작업 내용

1️⃣ 중복 url 처리 (InputCategoryUrl 컴포넌트)

• InputCategoryUrl 컴포넌트에서는 중복 처리를 위해 totalUrlInfos나 rightModalUrlInfos를 props로 받아 입력창으로 받은 url과 비교하여 중복을 처리하고 있는데, 이를 누락해서 발생한 오류 -> currentUrlInfos={totalUrlInfos} 추가

//ModalAddCategory.tsx

<InputCategoryUrl
	currentUrlInfos={totalUrlInfos}
	variant="basic"
	onUrlInputChange={(url: string) => handleUrlInputChange(url)}
/>

2️⃣ 에러메세지 표시 후, 에러메세지와 input창 초기화 안 되던 문제 해결

• 중복 에러를 확인하다가, 모달창을 나갔다 들어와도 에러메세지와 input창이 그대로 남아있는 오류를 발견

• 일단은 InputCategoryUrl 컴포넌트 내에서 useEffect를 사용해서 3초 뒤에 저절로 초기화되도록 구현했습니다..! 다른 방안이 있으시면 제안해주세요..

//InputCategoryUrl.tsx

useEffect(() => {
		if (isUrlValidated === false) {
			const timer = setTimeout(() => {
				setUrl('');
				setIsUrlValidated(true);
			}, 3000);

			return () => clearTimeout(timer);
		}
	}, [isUrlValidated]);

3️⃣ ButtonCategoryCommon 리팩토링

• 기존 ButtonCategoryCommon 안에 들어있던 데이터 clear 함수와 모달 창 나가는 함수들을 삭제 -> 필요한 이벤트핸들러는 밖에서 정의하여 이 컴포넌트를 사용할 때 그대로 전달해주기
• 이미 ButtonHTMLAttributes를 확장하여 인터페이스를 정의하고 있기 때문에, onClick, disabled 등의 속성은 ...props로 받아서 기본적인 HTML 버튼 속성들을 그대로 유지하고 있음 -> disabled, onClick 프로퍼티로 받아오지 않고 컴포넌트를 사용할 때 기본적인 버튼 속성 및 이벤트 핸들러를 그대로 전달할 수 있음

//ButtonCategoryCommon.tsx

	return (
		<button className={`${styledBtn} ${commonStyle}`} {...props}>
			{children}
		</button>
	);

4️⃣ CategoryModalRight에서 확인/취소 버튼을 눌렀을 때, CategoryModalLeft 초기화되도록 해결

• 일단 left와 right 모달의 관심사를 분리하기 어렵다는 판단 하에 AddCategoryListModal에 병합했습니다..!
• categoryId로 모달 왼쪽부분의 mset 데이터를 받아오고 있기 때문에, 취소/버튼 클릭시 categoryId를 0으로 변경하여 모달 왼쪽 부분의 mset 데이터를 초기화했습니다.

<ButtonCategoryCommon
	variant="취소"
	onClick={() => {
		handleClearModalData();
		handleClose();
		setCategoryId(0);
				}}
>
	취소
</ButtonCategoryCommon>
<ButtonCategoryCommon
	variant="완료"
	onClick={() => {
		handleClearModalData();
		handleSubmitModal();
		setCategoryId(0);
				}}
>
	완료
</ButtonCategoryCommon>

5️⃣ modal left, right 컴포넌트 병합 및 TitleMoribSet 컴포넌트 생성

• 모달의 left와 right 컴포넌트를 병합하니 AddCategoryListModal 컴포넌트 안의 코드가 길어져 가독성이 안좋아보여서, 복잡성을 줄이기 위해 원래 스타일적 요소만 들어있다는 이유로 해체했던 TitleMoribSet를 다시 분리했습니다.

type TitleMoribSetProp = { moribSetName: string };
const TitleMoribSet = ({ moribSetName }: TitleMoribSetProp) => {
	return (
		<div className="subhead-bold-22 mb-[8px] flex w-full flex-row justify-start p-[1rem]">
			<h2 className="text-mint-01">
				{moribSetName.length > 0 ? (
					moribSetName
				) : (
					<>
						<h2 className="pr-[1rem]" /> _______ <span className="pr-[0.5rem]" />
					</>
				)}
			</h2>
			<h2 className="text-white">의 모립세트</h2>
		</div>
	);
};

export default TitleMoribSet;

🧐 새로 알게된 점

• ButtonHTMLAttributes로 인터페이스 확장해서 ...props로 기본 html 버튼 속성 그대로 받아오는것.. 제가 짰지만 드디어 제대로 이해했습니다 하핫

🤔 궁금한 점

1. 위에서 에러메세지 표시 후, 에러메세지와 input창 초기화 안 되던 문제 InputCategoryUrl 컴포넌트 내에서 useEffect 사용했는데, 그냥 모달 최상단에서 inputUrl 상태와 isUrlValidated 상태를 관리하여 취소/완료 버튼 클릭시 상태 초기화되도록 하는 방법도 생각해봤습니다.
   근데 props 내려주기도 번거로울 것 같고, 버튼 클릭하기 전까진 에러메세지랑 Input창 내용이 계속 남아있으며, isUrlValidated 상태를 최상단에서 관리하는게 맞나 싶기도 해서 useEffect로 구현했습니다.. 더 나은 방법이 있을까요...?

2. 컴포넌트 분리 관련해서,, 모달 left와 right를 합치고, 저번 리팩토링때도 스타일적인 부분만 들어있는 컴포넌트는 모두 삭제 후 병합했는데, 한 컴포넌트 안에 내용이 너무 길어진 것 같긴 합니다.. 의견 있으시면 리뷰 달아주세요!

3. 컴포넌트들을 병합하고 시맨틱 태그로 구조파악을 용이하게 하려고 리팩토링 했었는데, 모달 창 특성상 header, footer 등으로 나누기 애매한 부분이 있더라고요..! 그부분도 일단 그냥 div로 수정했습니다.

4. 카테고리 추가 모달과 빠른 불러오기 모달의 모달 close 하는 함수가 다르게 돼있는데, 네이밍도 헷갈리고 handleClose 함수들이 구분이 잘 안돼서 상아꺼 ModalWrapper 머지 후 리팩토링할 수 있는 부분이 있으면 하겠습니다.

📸 스크린샷 / GIF / Link

앗 또 커밋 뒤에 번호 붙이는 것 깜빡해서 죄송합니다....유의하겠습니다...!!!!

[KIMGEONHWI]

고민의 흔적과 노력이 많이 담겨 있어서 인상 깊었습니다! pr도 꼼꼼하게 작성해주어서 이해하는데 도움 많이 되었습니다. 추가적으로 모달창을 띄우고 나서 esc를 누르면 해당 모달창이 사라지는데, esc는 입력란을 reset해주지 않고 모달을 잠깐 숨겨두게 할 것인지 혹은 esc 또한 기존 입력란에 입력된 것이 있으면 모두 reset되게 해줄 것인지 이야기해봐야 될 것 같아요. 고생하셨습니다!

[KIMGEONHWI]

ModalAddCategory 컴포넌트

const ModalAddCategory = ({ handleCloseModal }: ModalAddCategoryProps) => {
	const [resetKey, setResetKey] = useState(0); // 리셋을 위한 키 상태 추가
	const [totalUrlInfos, setTotalUrlInfos] = useState<UrlInfo[]>([]);
	const [rightModalUrlInfos, setRightModalUrlInfos] = useState<UrlInfo[]>([]);

const handleClearData = () => {
		setName('');
		setRightModalUrlInfos([]);
		setTotalUrlInfos([]);
		handleClearDateInfo();
		handlePeriodEnd();
		setResetKey((prevKey) => prevKey + 1); // key를 변경하여 컴포넌트를 리마운트
	};

<InputCategoryUrl
						key={resetKey}
						currentUrlInfos={totalUrlInfos}
						variant="basic"
						onUrlInputChange={(url: string) => handleUrlInputChange(url)}
					/>

위와 같은 방식으로 useEffect를 사용하지않고, 모달이 닫힐 때 InputCategoryUrl이 언마운트되도록하고 언마운트되면 상태가 초기화될 수 있게하는 방식은 어떨까요? 로컬에서 초기화 잘되는 것 확인했습니다.

[KIMGEONHWI]

https://ko.react.dev/learn/preserving-and-resetting-state
리액트 공식문서에 key를 이용해 자식 컴포넌트를 초기화해주는 내용 관련해서 참고하면 좋을듯합니다.

[suwonthugger]

p1: 이 부분은 InputUrl의 isUrlValidated 상태를 선언할 필요 없이, InputUrl의 에러 상태를 외부에서 주입하여, useEffect 없이 처리할 수 있을것 같아요.

먼저 최삼단에서 상태를 선언해서 prop으로 내려주어야할까? 내부에서 처리해야할까에 대한 부분이 고민이신것 같아요. 이러한 부분은 필요성에 대해서 먼저 생각을 해보면 좋을것 같아요.

제가 생각한 과정을 말씀드리자면,

• 먼저 모달(외부)을 닫을 때, InputUrl의 에러 상태가 초기화 되어야한다
• 즉 에러 상태는 InputUrl의 외부에서 제어가 가능해야한다.
• 이에 따라 InputUrl의 isUrlValidated와 관련된 에러 상태는 내부에서 선언되는게 아니라 외부에서 선언되고 관리되어야한다.
• 내부 isUrlValidated상태를 삭제하고, 이와 관련된 에러 상태를 InputUrl 컴포넌트 외부에서 선언하여, InputUrl에서는 이것을 prop으로 받아 에러 관련 메시지를 관리한다.

useEffect로 3초뒤에 동작하는 방식은, 어림짐작해서 동작하는 타이밍을 정한것이기 때문에 의도한 대로 동작이 되지 않을 가능성이 높습니다. 왜냐면 정확히 에러가 발생한 시점에 대해서 처리하는 방식이 아니기 떄문입니다!

건휘님이 공유해주신 문서에서 리액트의 자세한 동작방식을 살펴볼수 있었어요 감사드립니다. 하지만 이러한 상황에서는 키 값 대신 직관적으로 error인지 판단할수 있는 에러 상태를 prop으로 전달받아 에러 관련 UI를 업데이트 해주는것이 더 적합하지 않나 생각됩니다 🥷

[KIMGEONHWI]

p1: 이 부분은 InputUrl의 isUrlValidated 상태를 선언할 필요 없이, InputUrl의 에러 상태를 외부에서 주입하여, useEffect 없이 처리할 수 있을것 같아요.

먼저 최삼단에서 상태를 선언해서 prop으로 내려주어야할까? 내부에서 처리해야할까에 대한 부분이 고민이신것 같아요. 이러한 부분은 필요성에 대해서 먼저 생각을 해보면 좋을것 같아요.

제가 생각한 과정을 말씀드리자면,

• 먼저 모달(외부)을 닫을 때, InputUrl의 에러 상태가 초기화 되어야한다
• 즉 에러 상태는 InputUrl의 외부에서 제어가 가능해야한다.
• 이에 따라 InputUrl의 isUrlValidated와 관련된 에러 상태는 내부에서 선언되는게 아니라 외부에서 선언되고 관리되어야한다.
• 내부 isUrlValidated상태를 삭제하고, 이와 관련된 에러 상태를 InputUrl 컴포넌트 외부에서 선언하여, InputUrl에서는 이것을 prop으로 받아 에러 관련 메시지를 관리한다.

useEffect로 3초뒤에 동작하는 방식은, 어림짐작해서 동작하는 타이밍을 정한것이기 때문에 의도한 대로 동작이 되지 않을 가능성이 높습니다. 왜냐면 정확히 에러가 발생한 시점에 대해서 처리하는 방식이 아니기 떄문입니다!

건휘님이 공유해주신 문서에서 리액트의 자세한 동작방식을 살펴볼수 있었어요 감사드립니다. 하지만 이러한 상황에서는 키 값 대신 직관적으로 error인지 판단할수 있는 에러 상태를 prop으로 전달받아 에러 관련 UI를 업데이트 해주는것이 더 적합하지 않나 생각됩니다 🥷

좋은 의견 감사합니다!

[seueooo]

좋은 의견 감사합니다! InputUrl 컴포넌트 외부에서 에러 상태를 관리하도록 수정하겠습니다.

[KIMGEONHWI]

코드도 훨씬 깔끔해지고 공통컴포넌트로서의 역할이 더욱 명확해졌네요👍

[Ivoryeee]

공통 컴포넌트로서 활용성이 더 좋아진 것 같네요! 너무 좋습니다!

[suwonthugger]

• 카테고리 추가 모달과 빠른 불러오기 모달의 모달 close 하는 함수가 다르게 돼있는데, 네이밍도 헷갈리고 handleClose 함수들이 구분이 잘 안돼서 상아꺼 ModalWrapper 머지 후 리팩토링할 수 있는 부분이 있으면 하겠습니다. -> 넵!

• 현재 고쳐야할 버그가 또 있습니다.

  • 빠른 불러오기 모달(AddCategoryListModal) 들어갔다 나가면 상위 모달의 URL도 초기화가 됨

  • 빠른 불러오기 모달에서 중복을 검사할 때 상위 모달의 상태도 포함해서 검사해야하는 점

• 현재 모달 close 하는 함수가 너무 복잡한 상태라고 하셔서, 추가적으로 고쳐야할 버그는 상아 pr 추가 되고 하는게 좋을것 같다고 생각이 되긴합니다. 어떻게 하는게 좋을것 같으신가요,,,?

고생많으셨습니다~~ pr 자세히 작성해주셔서 감사ㄷ려요~~

[suwonthugger]

p1: 이 부분은 InputUrl의 isUrlValidated 상태를 선언할 필요 없이, InputUrl의 에러 상태를 외부에서 주입하여, useEffect 없이 처리할 수 있을것 같아요.

먼저 최삼단에서 상태를 선언해서 prop으로 내려주어야할까? 내부에서 처리해야할까에 대한 부분이 고민이신것 같아요. 이러한 부분은 필요성에 대해서 먼저 생각을 해보면 좋을것 같아요.

제가 생각한 과정을 말씀드리자면,

• 먼저 모달(외부)을 닫을 때, InputUrl의 에러 상태가 초기화 되어야한다
• 즉 에러 상태는 InputUrl의 외부에서 제어가 가능해야한다.
• 이에 따라 InputUrl의 isUrlValidated와 관련된 에러 상태는 내부에서 선언되는게 아니라 외부에서 선언되고 관리되어야한다.
• 내부 isUrlValidated상태를 삭제하고, 이와 관련된 에러 상태를 InputUrl 컴포넌트 외부에서 선언하여, InputUrl에서는 이것을 prop으로 받아 에러 관련 메시지를 관리한다.

useEffect로 3초뒤에 동작하는 방식은, 어림짐작해서 동작하는 타이밍을 정한것이기 때문에 의도한 대로 동작이 되지 않을 가능성이 높습니다. 왜냐면 정확히 에러가 발생한 시점에 대해서 처리하는 방식이 아니기 떄문입니다!

건휘님이 공유해주신 문서에서 리액트의 자세한 동작방식을 살펴볼수 있었어요 감사드립니다. 하지만 이러한 상황에서는 키 값 대신 직관적으로 error인지 판단할수 있는 에러 상태를 prop으로 전달받아 에러 관련 UI를 업데이트 해주는것이 더 적합하지 않나 생각됩니다 🥷

[suwonthugger]

p5: 컴포넌트 구성에 대해서 고민이 많으시군요. 필요에 따라서 컴포넌트를 유동적으로 분리하여 사용하면 좋을것 같아요. 재사용성만 생각했을 때 컴포넌트를 굳이 분리 안해도 될것 같지만 내가, 또는 협업자가 봤을 때 어느 역할을 하는지 명확히 하기위해 분리하는것도 정말 좋다고 생각해요.

예시로 한서님 코드를 변경해보자면,

<Container>
  <Header/>

  <ListUrl variant='left'>
      {data.map((item) => <ItemUrl {...item}/>
  <ListCategory/>
  
  <ListUrl variant='right'>
      {data.map((item) => <ItemUrl {...item}/>
  <ListCategory/>

<Container/>

이렇게 완벽하게 컴포넌트를 분리하기 쉽지 않지만, 위와 같이 분리한다면 협업자가 봐도 어떤 내용이 있는지 명확하게 파악이 가능할거에요. 이처럼 직관적으로 코드를 알아보기 쉽게 하는것도 좋은 근거라고 생각합니다.

제가 한 얘기들과 관련이 크게 없지만 토스 컴포넌트 관련 영상 링크 첨부해드릴게요.
https://www.youtube.com/watch?v=fR8tsJ2r7Eg&t=1s

[seueooo]

좋은 의견 감사합니다! 역할을 명확히 하기 위한 용도만으로도 컴포넌트를 분리하는 방법도 가독성에 도움이 되겠네요. 저도 더 고민해보고 유동적으로 적용해보도록 하겠습니다!

[Ivoryeee]

컴포넌트 구분에 대한 고민 과정이 드러나서 좋았습니다! 다른 분들이 언급해 주신 부분들 이외에 코드 가독성이 많이 개선된 것 같아 좋았습니다! 고생 많으셨어요! :-)

[Ivoryeee]

공통 컴포넌트로서 활용성이 더 좋아진 것 같네요! 너무 좋습니다!

[suwonthugger]

이번 작업 일단 머지하고, 상아가 만든 컴포넌트 사용해서 후속 테스크 진행부탁드려요!