Quick menu, 모임 카드 구현 및 폴더구조 리팩토링 #968

j-nary · 2024-12-16T23:04:53Z

🚩 관련 이슈

close 홈탭 캐러셀 / 카드 / 메뉴 #967

📋 작업 내용

Quick menu - Desktop 구현
Quick menu - Tablet 구현
모임 Card - Desktop 구현
폴더구조 리팩토링

📌 PR Point

폴더구조 리팩토링

기존 src/components/page 가 특정 페이지에서 쓰이는 컴포넌트들이 모여있는 폴더였는데, 좀 더 가시성을 확보하기 위해 pages 폴더와 구조를 같이 하여 도메인 별로 컴포넌트들을 분리할 수 있도록 리팩토링하였습니다.
그 외에 여러 도메인에 걸친 컴포넌트라면 src/components 폴더 내부에 생성하고, 도메인에 상관없는 컴포넌트라면 src/components/@common 폴더 내부에 생성하면 좋을 것 같다고 생각하였습니다.
해당 폴더구조 리팩토링은 당근페이 FE, 4개의 프로젝트를 하나로 합치며 나눈 대화들 을 참조하였습니다.
예를 들어, 원형 프로필 사진을 보여주는 컴포넌트 Avatar는 src/components/@common/Avatar/index.tsx 에, 특정 유저의 orgId 받아와서 Avatar 컴포넌트를 사용해 커스텀된 프로필 사진을 보여주는 컴포넌트 MemberAvatar는 src/components/MemberAvatar/index.tsx 에, 특정 페이지에서만 이 MemberAvatar를 겹쳐서 사용하는 컴포넌트 PostMemberAvatar는 src/components/page/post/PostMemberAvatar/index.tsx 에 위치시켜주시면 됩니다! (예시를 잘 든 건지 모르겠네요 ..)
현재, components 폴더 내부에 폴더명들이 PascalCase와 camelCase가 혼재되어있는데 깃이 대소문자를 구분하지 못해서 이전까지 있던 모든 것을 PascalCase로 바꾸기엔 위험성이 있다고 판단하여 바꾸진 않았습니다. 다만, 앞으로 만들어갈 컴포넌트의 폴더명은 PascalCase로 통일하는게 어떤지에 관해 이야기 나누고 싶습니다!

📸 스크린샷

2024-12-18.3.13.33.mov

height · 2024-12-16T23:04:56Z

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

borimong · 2024-12-18T03:54:19Z

src/components/Tab/index.tsx

+const CrewTab = ({ children }: { children?: ReactNode }) => {
+  const path = useRouter().pathname;
+  const lastSegment = path.split('/').at(-1);
+  console.log({ path });


콘솔 발견! 지워주시면 감사하겠습니다~

헉 감사합니다!

borimong · 2024-12-18T04:09:17Z

src/components/page/home/HomeCardList/CardList.tsx

+        {isMore && <SMoreBtn onClick={onMoreClick}>{'더보기 >'}</SMoreBtn>}
+      </STitleWrapper>
+      <SCardWrapper>
+        {data.map(d => (


data는 로딩중일 때는 undefined 일 수 있어서 옵셔널 체이닝을 걸어 undefined 참조 에러를 막아주면 어떨까요~??

애초에 undefined 이면 해당 컴포넌트로 넘어올 수 없게 아래와 같이 HomeCardList를 호출해주는 부분에서 분기처리해주었습니다! data가 로딩중이어서 undefined 일 수 있는 부분은 skeleton 등으로 부모 쪽에서 처리해주는 편이 좋다고 생각해서요! 이에 대해 어떻게 생각하시는지 의견 궁금합니다!

// pages/index.tsx <Flex justify="center"> {groupBrowsingCardData && ( <HomeCardList groupBrowsingCardData={groupBrowsingCardData as GroupBrowsingCardResponse} /> )} <QuickMenu /> </Flex>

앗 위에서 해주고 있었군요~! 제가 이 부분을 못 봣네요ㅎㅎ 사실 저는 이렇게 분기처리를 해준다고 해도, 서버로부터 받아오는 객체에 직접 접근할 때는 옵셔널 체이닝을 이중으로 하는 걸 선호하긴 해요! 코드가 나중에 바뀌어서 저 && 부분이 없어질지도 모르는데, 아예 객체 접근할 때 옵셔널 체이닝을 거는 것을 하나의 룰로 걸어두면, 코드 변경에 상관없이 undefined 참조 에러를 방지할 수 있어서요!! :-)

HomeCardList 의 prop으로 undefined를 허용하지 않아서 나중에 저 && 부분이 없어진다고 하더라도 컴파일타임에서 타입에러가 발생할 거에요! 데이터 로딩 여부는 부모 컴포넌트에서 관리하고, 하위 컴포넌트(UI 컴포넌트)는 정상적인 데이터가 전달될 때만 렌더링되는 것이 더 책임을 명확히 분리하는 좋은 방법이 될 수 있다고도 생각합니다!
모든 곳에 옵셔널 체이닝을 적용한다면 중복 방어 로직이 생겨서 컴포넌트 책임 분리가 어려울 것 같다고도 생각하는데 이에 대해서는 어떻게 생각하시는지 궁금합니다! :)

저도 옵셔널 체이닝 많이 사용하는 건 선호하지 않아서 (실제로 많이 사용했다가, 개발 당시에는 편했지만 버그를 찾아내는데 한 세월이 걸렸어서..) 부모 컴포넌트에서 &&로 처리해준 것 너무 좋은 것 같아요!

이중으로 옵셔널 체이닝하는 게 추후 코드 변경에 상관없이 참조 에러를 막을 수 있다는 것에도 동의하지만, 애초에 코드 변경이 생겼을 때 -> undefined로 경고를 확인할 수 있어서(= 컴포넌트 책임 분리가 잘 되어서) 저는 현재 구현된 방식을 더 선호하긴 합니다 :)

borimong · 2024-12-18T04:10:12Z

src/components/page/home/HomeCardList/CardList.tsx

+  color: '$gray200',
+});
+
+const SCardWrapper = styled('div', {


이렇게 간단한 flex 들은 mui 나 chakra ui 의 Flex 를 활용하면 좀 더 간편하게 작성할 수 있어!

좋은 피드백 고마워! 이 부분은 gap 이 필요한 부분이어서 유지하고, 앞으로 Flex 를 애용해볼게 !

chakra ui 기준으로
<Flex flexDir="row" columnGap="48px" alignItems="center">
와 같이 gap 도 줄 수 있어!ㅎㅎ

근데 우리 chakra ui 라이브러리를 사용했던가 ...? package.json에선 확인되지 않네 ..!

borimong · 2024-12-18T04:10:42Z

src/components/page/home/HomeCardList/index.tsx

+  return (
+    <div>
+      <CardList label="🔹 우리... 같이 솝커톤 할래?" isMore data={groupBrowsingCardData.slice(0, 3)} />
+      <CardList label="🔥 지금 모집중인 모임" data={groupBrowsingCardData.slice(0, 3)} />


옵셔널 체이닝 마찬가지 이유로 넣어주면 좋을 것 같은데 어때유~?

위와 마찬가지 이유입니다!

borimong · 2024-12-18T04:20:50Z

src/components/page/home/QuickMenu/index.tsx

+
+  paddingLeft: '106px',
+
+  '@media (max-width: 1259px)': {


우리 stitches.config.ts 파일 들어가보면, 반응형 미디어쿼리 설정해둔 것 볼 수 있어!

media: { small_mobile: '(max-width: 375px)', mobile: '(max-width: 414px)', tablet: '(max-width: 768px)', laptop: '(max-width: 1259px)', // default is desktop },

이 경우는 1259px 이니까 @Laptop 해주면 더 가독성 잇겠다! ㅎㅎ media 쿼리를 직접 쓰는 경우는, 이 기본 설정으로 커버되지 않을 때로 제한 하면 더 통일성 있을 것 같은데 진이 생각은 어때~?

좋아 반영할게오 !!

borimong · 2024-12-18T04:21:22Z

src/components/page/home/QuickMenu/index.tsx

+  padding: '20px 36px',
+  height: 'fit-content',
+
+  borderRadius: '9999px',


헉 borderRaidus 9999px 무슨 일이야 ㅋㅋㅋㅋㅋㅋㅋ (완전 원을 만들고 싶었던 건가???)

피그마대로 적용한거야!

chakra ui 의 Flex 를 styled 의 인자로 넣구, Flex 의 props 로 borderRadius="full" 을 넣어주면 조금 더 깔끔하게 코드를 바꿀 수 있을 것 같애!

display 속성은 모두 없어질 테니까!!

그리구 justifycontent 와 alignitems 모두 center 라면,
flexType: 'center',
을 이용해서 좀 더 간단하게 쓸 수 있어~! 사소한 부분이지만 도움이 됐기를 바라며~!!ㅋㅋㅋㅋㅋ

borimong · 2024-12-18T04:27:59Z

pages/index.tsx

@@ -3,11 +3,10 @@ import { useQueryGetGroupBrowsingCard } from '@api/API_LEGACY/meeting/hooks';
 import { useInfinitePosts } from '@api/post/hooks';
 import Carousel from '@components/groupBrowsing/Carousel/Carousel';
 import GroupBrowsingSlider from '@components/groupBrowsingSlider/groupBrowsingSlider';
-import DesktopFeedListSkeleton from '@components/page/meetingDetail/Feed/Skeleton/DesktopFeedListSkeleton';
-import MobileFeedListSkeleton from '@components/page/meetingDetail/Feed/Skeleton/MobileFeedListSkeleton';
-import RenderPostsWithAds from '@components/page/meetingList/Advertisement';


요게 없어졌네..! 그럼 피드 카드는 HomeCardList 가 보여주게 되는 거구 광고는 어디에서 보여주는 것일까요!??

HomeCardList 에 들어갑니다! 이제 피드 카드 전체가 사라져서 RenderPostsWithAds 컴포넌트가 필요없는 상황이에요! Ads 관련하여 코드 긁어올 부분 나중에 긁어오려고 컴포넌트 자체를 삭제하진 않았는데, 홈 탭 구현 마무리하면서 레거시 코드가 되지 않도록 삭제해두겠습니다!
Skeleton도 모양이 달라져야할 것 같아요! UI 자체가 아예 개편되어서!

borimong · 2024-12-18T04:28:54Z

pages/index.tsx

@@ -16,6 +15,9 @@ import Link from 'next/link';
 import { useEffect } from 'react';
 import { useInView } from 'react-intersection-observer';
 import { styled } from 'stitches.config';
+import { GroupBrowsingCardResponse } from '@api/API_LEGACY/meeting';
+import CrewTab from '@components/Tab';


컴포넌트 분리 최고~! 가독성 훨 좋아졌다!

borimong · 2024-12-18T04:35:08Z

pages/index.tsx

-              <Link href="/list">
+          </SContentTitle>
+          {groupBrowsingCardData && <GroupBrowsingSlider cardList={groupBrowsingCardData}></GroupBrowsingSlider>}
+          <SContentTitle style={{ marginBottom: '0px' }}>최신 피드</SContentTitle>


요 스타일은 인라인 형식으로 적어야할 이유가 있었을까요~??

이 부분은 원래부터 저렇게 되어있었어! 그리구 최신 피드 없어질 거라 아직 수정 안 한 부분이어요!

borimong · 2024-12-18T04:36:37Z

pages/index.tsx

+          <SCarouselBlank />
+          <Flex justify="center">
+            {groupBrowsingCardData && (
+              <HomeCardList groupBrowsingCardData={groupBrowsingCardData as GroupBrowsingCardResponse} />


HomeCardList 가 피드 카드를 보여주는 리스트 컴포넌트라면, props 를 groupBrowsingCardData 가 아닌 다른 이름으로 하면 좋을 것 같아요! groupBrowsingCardData의 경우에는 모임 둘러보기 캐러셀 데이터여가지구, 헷갈릴 수 잇다는 생각이 들어서요!! 진이 생각은 어떨지 궁금합니다~!

피드 카드가 아니라 홈에 위치할 각종 모임 카드 보여주는 리스트 컴포넌트입니다! groupBrowsingCardData는 임시로 넣은 거여서 나중에 홈 모임 API가 완성된다면, 추후에 API 연결하면서 바꿀 예정이었어요 !
이제 홈에서 피드 카드를 보여줄 일이 없게 되어서 !!

borimong · 2024-12-18T04:44:33Z

src/components/Tab/index.tsx

+  const tabText =
+    lastSegment === 'group'
+      ? 'feedAll'
+      : lastSegment === 'list'
+      ? 'groupAll'
+      : lastSegment === 'mine' || lastSegment === 'management'
+      ? 'mine'
+      : 'feedAll';


요것은 상수 객체로 별도로 저장해놓고, 쓸 때 TABTEXT[lastSegment] 이렇게 접근하면 좀 더 깔끔할 것 같은데 어떤가요~??

좋은 피드백 감사합니다! 반영하겠습니다!

ocahs9

좀 더 생산적인 방식을 찾아서 폴더 구조 리팩토링 한 거 너무 좋습니다!
첨부해주신 아티클도 전부 읽고, 현재 폴더 구조를 보는데 잘 반영된 것 같네요 :)

다만, 작업을 하다가 폴더 구조 개선을 해야겠다라고 생각을 하신 상황이라서 약간의 기능 구현과 폴더 구조 개선이 같이 섞여 있어 pr의 분리성이 떨어지는 것이 조금 아쉽긴 하네요 ㅜㅜ

예를 들어
import { TabList } from '@components/tabList/TabList';
가 많이 delete 되었던데(폴더 위치가 변한게 add되지 않고), TabList 자체가 더이상 쓰이지 않아서(레거시라서) 삭제해버린건지
아니면 실수로 삭제한건지 저는 파악하기 조금 어려웠어요 ㅜㅜ

그런데 전체적으로 본인만의 의견을 가지고 리팩토링을 진행하고,
코드를 고민하게 작성한게 pr에 보여서 저도 생각을 정리하며 같이 고민해보는 시간을 가질 수 있었습니다!
좋은 시도였던 것 같아요. 생각보다 번거로울 수 있었을텐데 고생하셨습니다 ☺️

ocahs9 · 2024-12-19T14:53:35Z

src/components/page/home/HomeCardList/CardList.tsx

+        {isMore && <SMoreBtn onClick={onMoreClick}>{'더보기 >'}</SMoreBtn>}
+      </STitleWrapper>
+      <SCardWrapper>
+        {data.map(d => (


저도 옵셔널 체이닝 많이 사용하는 건 선호하지 않아서 (실제로 많이 사용했다가, 개발 당시에는 편했지만 버그를 찾아내는데 한 세월이 걸렸어서..) 부모 컴포넌트에서 &&로 처리해준 것 너무 좋은 것 같아요!

이중으로 옵셔널 체이닝하는 게 추후 코드 변경에 상관없이 참조 에러를 막을 수 있다는 것에도 동의하지만, 애초에 코드 변경이 생겼을 때 -> undefined로 경고를 확인할 수 있어서(= 컴포넌트 책임 분리가 잘 되어서) 저는 현재 구현된 방식을 더 선호하긴 합니다 :)

ocahs9 · 2024-12-19T15:01:05Z

src/components/page/detail/Information/InformationPanel.tsx

-import { TabList } from '@components/tabList/TabList';
+import { TabList } from '@components/@common/tabList/TabList';


import { TabList } from '@components/tabList/TabList' 는 다른 곳에서는 다 삭제되고,
여기서만 유일하게 폴더 위치 바뀐 버전으로 TabList가 들어가있더라구요! 혹시 TabList 는 더이상 사용되지 않는건가요?

Tab 공통 컴포넌트로 분리한 것입니다! CrewTab 내에서 사용된 걸 확인하실 수 있는데 이 부분이 맞으실까요 ?

j-nary added 9 commits December 14, 2024 19:18

feat: QuickMenu 구현

54ca771

style: tablet gap 조정

c7fca8c

refactor: domain에 따른 폴더구조 변경

e3b94aa

refactor: 아이콘 및 레이블을 객체 매핑 방식으로 개선

f2bea86

feat: Desktop 모임 카드 구현

c2a2382

refactor: Tab 공통 컴포넌트 분리

574d07d

refactor: 도메인 종속되지 않은 공통 컴포넌트 폴더 분리

f1ddb06

refactor: HomeCardList 분리

483d566

style: 모임신청 가이드 mds 미적용

ec222cf

j-nary self-assigned this Dec 16, 2024

j-nary requested review from borimong and ocahs9 as code owners December 16, 2024 23:04

j-nary had a problem deploying to development December 16, 2024 23:04 — with GitHub Actions Failure

pull-request-size bot added the size/XL label Dec 16, 2024

fix: CI 빌드 에러 해결

3608e41

j-nary temporarily deployed to development December 16, 2024 23:15 — with GitHub Actions Inactive

pull-request-size bot added size/XXL and removed size/XL labels Dec 16, 2024

borimong reviewed Dec 18, 2024

View reviewed changes

j-nary added 3 commits December 18, 2024 14:08

refactor: 1259px 미디어쿼리 -> @Laptop

f008a53

chore: 콘솔 삭제

47f76b5

refactor: tabText 3항 연산자 -> 객체 맵핑

113b857

j-nary temporarily deployed to development December 18, 2024 05:39 — with GitHub Actions Inactive

refactor: Flex 활용

33e98b0

j-nary had a problem deploying to development December 18, 2024 06:53 — with GitHub Actions Failure

fix: chakra가 없다! 롤백!

a16f6a5

j-nary temporarily deployed to development December 18, 2024 07:02 — with GitHub Actions Inactive

ocahs9 approved these changes Dec 19, 2024

View reviewed changes

j-nary merged commit 51258ec into develop Dec 22, 2024
1 check passed

j-nary deleted the feat/#967 branch December 22, 2024 09:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quick menu, 모임 카드 구현 및 폴더구조 리팩토링 #968

Quick menu, 모임 카드 구현 및 폴더구조 리팩토링 #968

j-nary commented Dec 16, 2024 •

edited

Loading

height bot commented Dec 16, 2024

borimong Dec 18, 2024 •

edited

Loading

j-nary Dec 18, 2024

borimong Dec 18, 2024

j-nary Dec 18, 2024 •

edited

Loading

borimong Dec 18, 2024

j-nary Dec 18, 2024

ocahs9 Dec 19, 2024

borimong Dec 18, 2024

j-nary Dec 18, 2024

borimong Dec 18, 2024

j-nary Dec 18, 2024

borimong Dec 18, 2024

j-nary Dec 18, 2024

borimong Dec 18, 2024

j-nary Dec 18, 2024

borimong Dec 18, 2024

j-nary Dec 18, 2024

borimong Dec 18, 2024

borimong Dec 18, 2024 •

edited

Loading

j-nary Dec 18, 2024

borimong Dec 18, 2024 •

edited

Loading

borimong Dec 18, 2024

j-nary Dec 18, 2024

borimong Dec 18, 2024

j-nary Dec 18, 2024

borimong Dec 18, 2024

j-nary Dec 18, 2024

ocahs9 left a comment •

edited

Loading

ocahs9 Dec 19, 2024

ocahs9 Dec 19, 2024

j-nary Dec 22, 2024

		import { TabList } from '@components/tabList/TabList';
		import { TabList } from '@components/@common/tabList/TabList';

Quick menu, 모임 카드 구현 및 폴더구조 리팩토링 #968

Quick menu, 모임 카드 구현 및 폴더구조 리팩토링 #968

Conversation

j-nary commented Dec 16, 2024 • edited Loading

🚩 관련 이슈

📋 작업 내용

📌 PR Point

폴더구조 리팩토링

📸 스크린샷

height bot commented Dec 16, 2024

borimong Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-nary Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borimong Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borimong Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocahs9 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-nary commented Dec 16, 2024 •

edited

Loading

borimong Dec 18, 2024 •

edited

Loading

j-nary Dec 18, 2024 •

edited

Loading

borimong Dec 18, 2024 •

edited

Loading

borimong Dec 18, 2024 •

edited

Loading

ocahs9 left a comment •

edited

Loading