Skip to content
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

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

Merged
merged 5 commits into from
Sep 23, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: 에러메세지 뜨고 3초 뒤 초기화 추가..
seueooo committed Sep 15, 2024
commit a249cb0ef30b0070744d706aa3033c12ec865d6c
10 changes: 5 additions & 5 deletions src/pages/HomePage/components/ModalAddCategory.tsx
Original file line number Diff line number Diff line change
@@ -161,7 +161,7 @@ const ModalAddCategory = ({ handleCloseModal }: ModalAddCategoryProps) => {

const handleClose = () => {
handleClearData();
handleCloseModal();
closeModal();
};

const handleCategoryModalClose = () => {
@@ -249,7 +249,7 @@ const ModalAddCategory = ({ handleCloseModal }: ModalAddCategoryProps) => {
</button>
<AddCategoryListModal
handleSubmitModal={handleMsetSubmit}
handleClose={handleCategoryModalClose}
handleClose={handleClose}
dialogRef={dialogRef}
rightModalUrlInfos={rightModalUrlInfos}
handleRightModalUrlInfos={handleRightModalUrlInfos}
@@ -273,14 +273,14 @@ const ModalAddCategory = ({ handleCloseModal }: ModalAddCategoryProps) => {
</CategoryCommonMoribSet>
</main>

<footer className="mt-[3rem] flex justify-end gap-[1.6rem]">
<ButtonCategoryCommon variant="취소" onClick={handleClose}>
<div className="mt-[3rem] flex justify-end gap-[1.6rem]">
<ButtonCategoryCommon variant="취소" onClick={handleCategoryModalClose}>
취소
</ButtonCategoryCommon>
<ButtonCategoryCommon variant="완료" onClick={handlePostDataClick} disabled={!isFormValid()}>
완료
</ButtonCategoryCommon>
</footer>
</div>
</>
);
};
4 changes: 2 additions & 2 deletions src/shared/components/AddCategoryListModal.tsx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
@@ -135,7 +135,7 @@ const AddCategoryListModal = ({
</CategoryCommonMoribSet>
</div>
<div className="flex h-[80rem] w-[61.2rem] flex-col items-end justify-between rounded-r-[1rem] bg-gray-bg-03 pb-[3rem] pl-[3rem] pr-[4.3rem] pt-[9.7rem]">
<header className="subhead-bold-22 mb-[8px] flex w-full flex-row justify-start p-[1rem]">
<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
@@ -146,7 +146,7 @@ const AddCategoryListModal = ({
)}
</h2>
<h2 className="text-white">의 모립세트</h2>
</header>
</div>

<InputCategoryUrl
currentUrlInfos={rightModalUrlInfos}
13 changes: 12 additions & 1 deletion src/shared/components/InputCategoryUrl.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';

import { isUrlValid } from '@/shared/utils/isUrlValid/index';

@@ -29,6 +29,17 @@ const InputCategoryUrl = ({ variant = 'basic', onUrlInputChange, currentUrlInfos
const defaultStyle = `subhead-med-18 h-[4.6rem] rounded-[8px] border-[1px] bg-gray-bg-02 px-[2rem] py-[1rem] text-white placeholder-gray-03 focus:outline-none ${sizeVariantWidth[variant]}`;
const borderStyle = isUrlValidated === false ? 'border-error-02' : 'border-transparent';

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

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

Comment on lines +32 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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이 언마운트되도록하고 언마운트되면 상태가 초기화될 수 있게하는 방식은 어떨까요? 로컬에서 초기화 잘되는 것 확인했습니다.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

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

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

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

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

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

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

좋은 의견 감사합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
if (e.key === 'Enter') {
const isValid = isUrlValid(url);