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 ] 모달 내 버그 해결 및 리팩토링 #200

Merged
merged 7 commits into from
Oct 8, 2024

Conversation

seueooo
Copy link
Collaborator

@seueooo seueooo commented Sep 25, 2024

🔥 Related Issues

✅ 작업 리스트

  • InputUrl 컴포넌트 외부에서 에러 상태를 관리하도록 수정
  • 빠른 불러오기 모달(AddCategoryListModal) 들어갔다 나가면 상위 모달의 URL도 초기화가 되는 오류 수정
  • 빠른 불러오기 모달에서 중복을 검사할 때 상위 모달의 상태도 포함해서 검사해야하는 점 수정
  • 드롭다운 useClickOutside 적용

🔧 작업 내용

1️⃣ InputUrl 컴포넌트 외부에서 에러 상태를 관리하도록 수정

  • inputUrl과 url이 유효인지 확인하는 isValidated 상태를 모달 최상단에서 관리하도록 수정 -> useEffect 삭제
  • 완료, 취소 버튼 클릭시 input창과 isValidated 상태 모두 초기화 되도록 수정

2️⃣ 빠른 불러오기 모달(AddCategoryListModal) 들어갔다 나가면 상위 모달의 URL도 초기화가 되는 오류 수정

  • 모달 close하는 함수 수정
  • 빠른 불러오기 모달의 취소 버튼 handleSecondModalClose() 함수에서 handleClearData 함수 삭제 후 해결
    첫번째 모달
  <ButtonCategoryCommon
	variant="취소"
	onClick={() => {
		handleClearData();
		handleCloseModal();
		}}
>
	 취소
</ButtonCategoryCommon>
<ButtonCategoryCommon
	variant="완료"
	onClick={() => {
		handlePostDataClick();
	}}
disabled={!isFormValid()}
>
       완료
</ButtonCategoryCommon>

두번째 빠른 불러오기 모달

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

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

  • Total url과 right modal의 url이 합쳐질 때 중복인 것은 제외되도록 다음과 같이 수정
const handleAddTotalUrl = () => {
		setTotalUrlInfos((prev) => {
			const existingUrls = new Set(prev.map((totalUrlInfo) => totalUrlInfo.url));

			const newUrls = rightModalUrlInfos.filter((rightUrlInfo) => existingUrls.has(rightUrlInfo.url) === false);

			return [...prev, ...newUrls];
		});
	};

4️⃣ 드롭다운 useClickOutside 적용

  • useClickOutside 적용해서 드롭다운 바깥쪽 클릭시 닫히도록 함

🧐 새로 알게된 점

🤔 궁금한 점

📸 스크린샷 / GIF / Link

  • 빠른 불러오기 모달(AddCategoryListModal) 들어갔다 나가면 상위 모달의 URL도 초기화가 되는 오류 해결
2024-09-27.5.04.41.mov
  • 빠른 불러오기 모달에서 중복을 검사할 때 상위 모달의 상태도 포함해서 검사해야하는 점 수정
    ( 슬래시/ 여부 주의..)
2024-09-27.5.06.36.mov
  • 에러메세지 뜬 채로 모달 창 나갔다 들어오면 에러메세지랑 input창 남아있던 오류 해결
2024-09-27.5.08.48.mov
  • 드롭다운 바깥 쪽 클릭시 드롭다운 닫히도록 함
2024-09-27.5.10.08.mov

@seueooo seueooo added 🐞 BugFix Something isn't working ♻️ Refactor 코드 리팩토링 labels Sep 25, 2024
@seueooo seueooo self-assigned this Sep 25, 2024
@seueooo seueooo linked an issue Sep 25, 2024 that may be closed by this pull request
4 tasks
Copy link
Member

@suwonthugger suwonthugger left a comment

Choose a reason for hiding this comment

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

이 모달은 삭제 예정이지만,,,,,

기디 친구들이 디벨롭을 하는 과정에서 기존 구현된 웹을 참고할 수 있을것 같아요. 그래서 간단하게 오류가 남아있는 부분 만 수정해봅시다. 너무 고생많으셨어요...!!!

src/pages/HomePage/components/ModalAddCategory.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@Ivoryeee Ivoryeee left a comment

Choose a reason for hiding this comment

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

기존 버그가 많이 해결됐네요 그동안 너무 수고 많으셨습니다!
모달 부분은 디벨롭 되더라도 현재 로직이나 공통 컴포넌트들이 잘 구현돼 있어 이 부분 잘 활용할 수 있을 것 같습니다!
3차 스프린트 때 같이 또 달려 보아요 !! :)

Copy link
Member

@suwonthugger suwonthugger left a comment

Choose a reason for hiding this comment

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

  • 아래 오류가 뜨는데 이 부분 한번만 확인해주세요!
    <h2> 태그 아래 <h2> 태그가 있어서 뜨는 오류같아요!
image

@seueooo
Copy link
Collaborator Author

seueooo commented Oct 7, 2024

  • 아래 오류가 뜨는데 이 부분 한번만 확인해주세요!
    <h2> 태그 아래 <h2> 태그가 있어서 뜨는 오류같아요!
image

넵 반영했습니다!

Copy link
Member

@suwonthugger suwonthugger left a comment

Choose a reason for hiding this comment

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

LGTM

@suwonthugger suwonthugger merged commit aa71938 into develop Oct 8, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 BugFix Something isn't working ♻️ Refactor 코드 리팩토링
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

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