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

feat: 공통 컴포넌트 gnb 컴포넌트 UI 작성 #26

Merged
merged 27 commits into from
Sep 9, 2024

Conversation

hakyoung12
Copy link
Contributor

@hakyoung12 hakyoung12 commented Sep 6, 2024

✏️ 작업 내용

  • gnb 컴포넌트 UI 작성

📷 스크린샷

스크린샷 2024-09-06 오후 4 16 39

PC사이즈

스크린샷 2024-09-06 오후 4 22 17

태블릿사이즈

스크린샷 2024-09-06 오후 4 21 45

모바일사이즈

스크린샷 2024-09-06 오후 4 24 46

로그아웃상태 레이아웃

스크린샷 2024-09-06 오후 4 23 48

모임찾기 활성화시(예시)

스크린샷 2024-09-06 오후 4 25 41

프로필 드롭다운

✍️ 사용법

//UserStatus.tsx
const user = {
  name: 'test name',
}; // 이부분을 null로 바꿔 로그아웃시 레이아웃을 볼 수 있습니다.

layout.tsx에서 사용될 UI입니다.

🎸 기타

  • 현재 pathname이 지정되지 않아 네비게이션바의 링크는 '#'로 되어있습니다. 정해지면 수정 예정입니다.

@hakyoung12 hakyoung12 added the ✨ Feat 기능 추가 label Sep 6, 2024
@hakyoung12 hakyoung12 self-assigned this Sep 6, 2024
];

const Gnb = () => {
const pathname = usePathname();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

현재 URL 경로를 가져오는 코드입니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

{navList.map((nav, index) => (
<li key={index}>
<Link
className={`text-[16px] font-semibold ${pathname.includes(nav.link) ? 'text-black' : 'text-white'}`}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

현재 URL경로를 받아와서 link가 포함되어있다면 글자색상이 검정색으로 출력됩니다.

Copy link
Contributor

@BeMatthewsong BeMatthewsong left a comment

Choose a reason for hiding this comment

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

학영님 코드 잘 짜시네요! LGTM👍

import { usePathname } from 'next/navigation';
import UserStatus from './UserStatus';

//@todo pathname 정해질 시 추가 예정
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

P4) 정말 사소한 건데 'use client' 쓰고 한 줄 띄워주셔도 좋을 것 같습니다!

Comment on lines +20 to +25
if (
dropDownRef.current &&
!dropDownRef.current.contains(event.target as Node)
) {
setIsOpen(false);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

P3) !dropDownRef.current.contains(event.target as Node) 에 대해 설명 부탁드려도 되나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

event.target, 즉 현재 클릭이벤트가 일어난 곳이 dropDownRef.current가 참조하는 DOM 요소의 내부에 있는지 아닌지 판단하고 만약 false가 나올 시에 setIsOpen(false);를 실행하여 드롭다운을 닫습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

객체 야무지게 잘 쓰셨네요 👍

Copy link
Contributor

@yunchaeney yunchaeney left a comment

Choose a reason for hiding this comment

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

👍👍👍

Comment on lines 36 to 41
<Link
className={`text-[16px] font-semibold ${pathname.includes(nav.link) ? 'text-black' : 'text-white'}`}
href={nav.link}
>
{nav.name}
</Link>
Copy link
Contributor

Choose a reason for hiding this comment

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

componenets/Tab/TopTab.tsx 파일에 해당 부분이 컴포넌트로 작성되어 있습니다만, 굳이 컴포넌트로 갈아끼울 필요가 있을까? 싶습니다.
현재 방식도 복잡해 보이진 않아서 유지하고, 위 언급한 컴포넌트는 아예 삭제해도 될 것 같아요.

p1) 모바일 사이즈에서 font 크기 조정이 필요합니다.
p2) text-[16px]text-16 으로 수정해 주시면, 제 PR 머지 시 반영됩니다.
--> 괜찮으시면 위 언급한 컴포넌트에서 class name 만 참고해 주세요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

text 사이즈는 수정완료했습니다!
제가 봤을 때에는 네이게이션바를 다른 컴포넌트로 분리하고 채현님께서 만든 컴포넌트를 사용하는 방식도 좋아보입니다!
사실 제 방식은 찜한 모임 뱃지 넣기 애매할거같더라구요...

Comment on lines +14 to +15
name: '찜한 모임',
link: '#',
Copy link
Contributor

Choose a reason for hiding this comment

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

찜한 모임 옆에 갯수 Badge 달리는 경우는 어떻게 되나요 ?_?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그부분은 제가 깜빡했습니다ㅠ 추가하겠습니다!

Copy link
Contributor

@HMRyu HMRyu left a comment

Choose a reason for hiding this comment

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

깔끔하게 잘 작성해 주신 것 같습니다!

작업 속도가 엄청 빠르신 것 같아요... 🤣

고생 많으셨습니다..!

Comment on lines 27 to 29
<header className='gap-0 bg-var-orange-600 py-16'>
<div className='mx-16 flex max-w-[1200px] items-center justify-between md:mx-24 lg:mx-auto'>
<nav className='flex items-center'>
Copy link
Contributor

Choose a reason for hiding this comment

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

시멘틱 태그 👍

P4) 혹시 gap-0 은 사용되고 있는 스타일일까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

잘못 들어간 스타일인데 현재는 수정했습니다!

import { usePathname } from 'next/navigation';
import UserStatus from './UserStatus';

//@todo pathname 정해질 시 추가 예정
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

P4) 정말 사소한 건데 'use client' 쓰고 한 줄 띄워주셔도 좋을 것 같습니다!

];

const Gnb = () => {
const pathname = usePathname();
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines +49 to +60
<ul className='flex gap-24'>
{navList.map((nav, index) => (
<li key={index} className='flex items-center gap-[5px]'>
<Link href={nav.link}>
<TopTab isActive={pathname.includes(nav.link)}>
{nav.name}
</TopTab>
</Link>
{renderBadge(nav.name)}
</li>
))}
</ul>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

9/9 변경사항입니다!

  • TopTab 컴포넌트 적용
  • 찜한 모임 뱃지 추가

Copy link
Contributor

Choose a reason for hiding this comment

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

👍👍👍 반영 감사합니다!

@hakyoung12 hakyoung12 merged commit 804abc1 into develop Sep 9, 2024
4 checks passed
@@ -0,0 +1,21 @@
interface ChipProps {

Choose a reason for hiding this comment

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

칩스 컴포넌트를 하나로 선언해두고 variant 프롭스로 InfoChip, StateChip, TimeChip 등 다양한 칩을 리터하도록 내부에서 조정해보도록 리팩토링해보시면 좋을 것 같습니다.

공통 컴포넌트 혹은 디자인 시스템을 사용할대, 아톤 컴포넌트 하나를 만들고 그 내부에서 variant props를 두고 사용목적에 맞게 리턴하도록 사용하는 패턴이 많이 응용되거든요

pending: 'bg-white text-var-gray-500 border border-var-gray-200',
};

const stateContents = {

Choose a reason for hiding this comment

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

상수로 분리해두는게 더 좋을 것 같네요


/* 찜한 모임 배지 렌더링 함수
nav.name이 '찜한 모임'이고 웹스토리지에 저장된 '찜한 모임'의 개수가 1개 이상일 시 Badge 렌더링 */
const renderBadge = (name: string) => {

Choose a reason for hiding this comment

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

목요일날 배운 뷰 컴포넌트에서 상태 로직 분리하기를 떠올려보시고,
커스텀 훅을 작성해보는게 어떨까요?


const UserStatus = () => {
const [isOpen, setIsOpen] = useState<boolean>(false);
const dropDownRef = useRef<HTMLDivElement>(null);

Choose a reason for hiding this comment

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

이러한 로직들도 하나의 공통 훅으로 마련해서 처리를 해보면 좋을 것 같네요

Choose a reason for hiding this comment

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

cont { ref, clickOutSide } = useClickoutSide();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feat 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants