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

[ListPage]🍝코드 정리 #594

Merged
merged 21 commits into from
Feb 19, 2024
Merged

[ListPage]🍝코드 정리 #594

merged 21 commits into from
Feb 19, 2024

Conversation

lydiacho
Copy link
Member

@lydiacho lydiacho commented Feb 15, 2024

🔥 Related Issues

resolved #590

💜 작업 내용

  • TattooCard, FilterButton 컴포넌트 분리
  • html 내 문자열 상수화
  • TattooList > selectedFilter state 및 useEffect 삭제
  • buttons state 추가하여 DEFAULT_BUTTONS_NAME 배열, buttonName state 삭제
  • selectedTag state 배열 => string 변경함으로써 및 로직 간소화
  • FilterSheet > filterTag state 및 관련 로직 삭제
  • 바텀시트 내 태그 선택 처리 : className toggle하는 로직 및 useRef 삭제 -> state에 따른 조건부 렌더링으로 변경하여 로직 간소화
    그 외 버그 수정
  • FilterSheet 렌더링 시 styled-componetns warning 3개 안뜨도록 수정
  • FilterSheet Backdrop 클릭 시 Backdrop 아래의 컴포넌트(타투 카드, 헤더 버튼 등)이 중첩 클릭되는 문제 해결

✅ PR Point

개인 리팩토링

  • state 개수 최소화 -> 최소한의 state를 활용해서 모든 기능을 구현할 수 있도록 state를 수정하고 삭제하였습니다
  • className을 활용한 토글 기능 -> state를 활용한 조건부 렌더링 방식으로 수정하여 불필요한 로직을 크게 줄였습니다
    이 외에도 useRef, useEffect 등 많이 삭제하면서 기존의 state간의 로직이 굉장히 복잡히 꼬여있었는데 이부분을 크게 정리했습니당

버그 수정

1️⃣ FilterSheet 렌더링 시 styled-componetns warning 3개 안뜨도록 수정

스크린샷 2024-02-15 오후 4 05 27

항상 바텀시트를 키면 저 세가지의 워닝이 콘솔에 떴었는데요, 거슬려서 없애주었습니다.
일반적인 Styled-Component는 저런 워딩이 뜰 경우 $prefix를 써서 해결해주는데요, 저는 react-modal-sheet 라이브러리가 제공하는 컴포넌트를 상속받아서 사용하는 경우라 마음대로 prefix를 붙일 수 없었습니다.

또 $prefix를 붙인다는 건 해당 props는 DOM 요소에 영향을 안끼치도록 필터링된다는 의미인데,
제 props는 스타일링이 아닌 기능을 해야하는 친구이기 때문에 Prefix는 부적절해요

따라서 최상위를 감싸는 div wrapper를 하나 더 만들어서 그곳에서 className으로 필요한 스타일링을 해주었고요,
라이브러리 컴포넌트는 상속받아서 커스텀 스타일링 추가하지 않고 그대로 사용하였습니다.
이러면 라이브러리 컴포넌트가 Styled Component로 여겨지지 않아서 더이상 워닝이 발생하지 않아요

  SheetWrapper: styled.div`
    .react-modal-sheet-backdrop {
      background-color: rgba(0, 0, 0, 0.6) !important;
    }
    .react-modal-sheet-container {
      border-radius: 1rem !important;
    }
    .react-modal-sheet-header {
      height: 1.6rem !important;
    }
    .react-modal-sheet-drag-indicator {
      display: none;
    }
  `,

( 커밋 changes 보기 )

2️⃣ FilterSheet Backdrop 클릭 시 Backdrop 아래의 컴포넌트(타투 카드, 헤더 버튼 등)이 중첩 클릭되는 문제 해결
스크린샷 2024-02-15 오후 4 30 43

이 바텀시트의 백드롭을 클릭하면 바텀시트가 닫히는 액션만 취해져야 하는데요,
TattooCard, 홈으로 이동하는 로고, 사이드바를 여는 햄버거 아이콘 등 dimmed된 바텀시트 아래에 있는 영역들이 함께 클릭이 돼서 실행되는 문제가 있었습니다.
예를들어 바텀시트가 켜져있는 상태에서 제가 햄버거 버튼 쪽을 터치하면,
바텀시트만 꺼져야하는데 바텀시트가 꺼지는 동시에 사이드바도 열리는 상황인거죠

처음에 이벤트 버블링 관련 이슈인줄 알고 stopPropagation 했다가 소용없어서 DOM트리를 뜯어보면서 열심히 고민했는데..
원인은 별거 아니었습니다

const onTapBack = () => {
    setSelectedTag(buttons[isSheetOpen].value);
    setSheetOpen(-1);  // 여기 
  };

백드롭을 눌렀을 때 실행되는 함수인 onTapBack 내에서 바텀시트를 종료하는 코드가 있는데요,
바텀시트가 꺼진 이후에도 제가 클릭한 이벤트가 유효해서 그 아래에 있던 친구들에게도 영향이 가는 문제였습니다.
이 onTap 리스너의 특징이 onClick리스너랑은 달리 motion.button 이벤트 리스너라서 아마 이벤트 지속이 더 오래 되는 것 같아요 (추측. 더 알아봐야함)

react-modal-sheet 라브에서 backdrop에 클릭함수 넣으려면 onCllick말고 onTap만 써야한다고 함

무튼 그래서 isSheetOpen이 -1일 때 발생하는 IndexError를 방지하기 위해 조건부렌더링을 해주고 있었는데요,
렌더링 영역을 라브컴포넌트 전체(시트+백드롭)이 아닌, 시트 내부로 변경함으로써 해결했습니다

스크린샷 2024-02-16 오전 5 27 45

이렇게 하면 바텀시트가 닫히는 애니메이션이 시작되고, setSheetOpen(-1)을 하고, 애니메이션이 끝날 떄까지 backdrop이 계속 렌더링되어있어 아래 컴포넌트들한테까지 영향이 가지 않도록 막아주겠죠 !

@lydiacho lydiacho added 🔨 Refactor 리팩토링 📑 List ListPage labels Feb 15, 2024
@lydiacho lydiacho self-assigned this Feb 15, 2024
Copy link

netlify bot commented Feb 15, 2024

Deploy Preview for tattour ready!

Name Link
🔨 Latest commit ad05e3b
🔍 Latest deploy log https://app.netlify.com/sites/tattour/deploys/65ce2a00a184d3000821bdee
😎 Deploy Preview https://deploy-preview-594--tattour.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@lydiacho lydiacho changed the title [Refactor] 🍝코드 정리 [ListPage]🍝코드 정리 Feb 15, 2024
Copy link
Member

@Yeonseo-Jo Yeonseo-Jo left a comment

Choose a reason for hiding this comment

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

수고하셨습니당~

별거는 아니지만 바텀시트 수정한김에
pc 화면에서 max width 줘서 바텀시트도 모바일 화면 크기로(꽉 차게 x) 보여지게 수정 해줘도 좋을것 같아요

image

⬆️ 요거!

Comment on lines 26 to +39

const DEFAULT_BUTTON_NAME = ['정렬', '장르', '스타일'];
const [isSheetOpen, setSheetOpen] = useState(-1); // -1이 바텀시트 off 상태

// state && : 선택된 필터가 있을 경우
// state.type이 장르일 때 버튼명을 state.name 값으로, 아니면 '장르'
// state.type이 스타일일 때 버튼명을 state.name 값으로, 아니면 '스타일'
const [buttonName, setButtonName] = useState([
'정렬',
`${state && type === '장르' ? name : '장르'}`,
`${state && type === '스타일' ? name : '스타일'}`,
const [buttons, setButtons] = useState<buttonType[]>([
{
default: SORT,
value: SORT,
},
{
default: GENRE,
value: state && type === GENRE ? name : GENRE,
},
{
default: STYLE,
value: state && type === STYLE ? name : STYLE,
},
Copy link
Member

Choose a reason for hiding this comment

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

buttons로 수정되면서 로직을 단순화 시킨게 너무 좋네요-!

@lydiacho
Copy link
Member Author

감사합니다! 해당부분은 별도 이슈 생성해서 따로 처리하도록 하겠습니다!

@lydiacho lydiacho merged commit 4af7200 into develop Feb 19, 2024
4 checks passed
@lydiacho lydiacho mentioned this pull request Feb 23, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📑 List ListPage 🔨 Refactor 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants