-
Notifications
You must be signed in to change notification settings - Fork 0
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
[JOJI-769] 반응형 네비게이션 #34
The head ref may contain hidden characters: "feat/JOJI-797/\uC18C\uAC1C\uC139\uC158-\uBC18\uC751\uD615"
Conversation
|
Warning Rate limit exceeded@MuteAim has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
헤이 헤이 Chill... Walkthrough이번 변경 사항은 Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/_components/navBar.js (2)
101-131
: 헤이 헤이 Chill 모바일 전용 메뉴 아이템 배열을 따로 두었네. 동일한 데이터를 중복 관리하면 수정할 때 실수할 여지도 있으니, 가능하면 공통 구조를 하나만 두고 뷰에서 분기하는 방법도 고민해봐도 좋을듯
257-385
: 헤이 헤이 Chill 데스크톱 섹션 로직이 모바일보다 꽤 복잡하네. 마우스오버 로직도 잘 처리했지만 함수 분리 등을 통해 좀 더 모듈화하면 코드 유지보수에 좀 더 chill하게 접근 가능해 보여
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json
(2 hunks)src/app/_components/navBar.js
(3 hunks)src/app/_components/sections/heroSection.js
(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/_components/navBar.js
[error] 191-257: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (11)
src/app/_components/navBar.js (5)
5-7
: 헤이 헤이 Chill 새로운 아이콘과 MUI 관련 임포트들이 들어왔네. 사용하는 아이콘이 프로젝트 스타일과 잘 어울려 보이니 좋음. 호환성 면에서도 크게 문제 없어 보이니 차분하게 가보자
19-20
: 헤이 헤이 Chill 모듈 상태관리 isMobileMenuOpen, openMenu 스테이트 추가된 거 괜찮음. 모바일용, 드롭다운용 상태를 명확히 구분했네. 딱히 문제 없어 보이니 코드는 chill하게 유지해봐
22-22
: 헤이 헤이 Chill useMediaQuery({ maxWidth: 768 }) 사용은 반응형 처리에 유용해 보임. Next.js에서 서버 사이드 렌더링 시점 관련 이슈가 없는지 한 번 정도 확인해두면 좋을 듯
95-95
: 헤이 헤이 Chill /post 경로로 직접 이동하도록 변경했네. 공지사항 메뉴를 좀 더 직관적으로 갈 수 있게 됨. 문제 없어 보이므로 chill하게 두는 게 좋겠어
133-135
: 헤이 헤이 Chill toggleMenu 함수가 간결하고 명확함. 메뉴 토글에 딱 필요한 로직만 들어가 있어서 무난하게 작동될 것 같네src/app/_components/sections/heroSection.js (4)
11-13
: 헤이 헤이 Chill 패딩과 간격을 좀 더 세분화했네. 모바일부터 데스크톱까지 유연하게 잡혔음. 레이아웃 균형도 좋아 보이니 chill하게 유지하면 됨
14-21
: 헤이 헤이 Chill 모바일 화면에서만 Card를 보여주는 아이디어가 사람들 접근성에 좋아 보임. 이미지가 가운데로 예쁘게 들어가서 눈에 잘 띌 듯. 안 부딪히는 듯 하니 chill해
24-24
: 헤이 헤이 Chill 텍스트 사이즈 조절과 gap 수정은 보기에 안정감 있어 보이네. 사용자가 데스크톱에서든 모바일에서든 부담 없이 볼 수 있을 것 같음Also applies to: 26-26, 31-31
50-51
: 헤이 헤이 Chill md 이상에서만 오른쪽 콘텐츠를 보여주는 방식 좋네. 반응형에서 콘텐츠 정돈에 도움이 될 듯package.json (2)
10-10
: 헤이 헤이 Chill dev 스크립트가 HTTPS와 443 포트로 바뀌었다는 건데, SSL 인증서 세팅 같은 부분은 별도로 확인해봐야 할 수도 있어. 로컬 환경에서 브라우저 관련 경고는 없는지 잘 체크해보자
18-18
: 헤이 헤이 Chill 새로 추가된 의존성들이 다 반응형, 아이콘, MUI 쪽이라 네비 개선과 스타일링에 잘 어울릴 것 같음. 설치 시 충돌 없이 잘 돌아가는지 빌드 한번 해봐도 좋을 듯Also applies to: 20-20, 35-35, 37-37, 38-38
src/app/_components/navBar.js
Outdated
</button> | ||
)} | ||
|
||
{/* 햄버거 메뉴 버튼 */} | ||
<button | ||
className="text-sm font-semibold text-muted-foreground" | ||
onMouseEnter={() => setIsUserDropdownOpen((prev) => !prev)} | ||
onClick={() => setIsMobileMenuOpen(!isMobileMenuOpen)} | ||
className="text-2xl" | ||
> | ||
환영합니다! {userName}님 | ||
{isMobileMenuOpen ? <FaTimes /> : <FaBars />} | ||
</button> | ||
{isUserDropdownOpen && ( | ||
<div | ||
className="absolute right-0 mt-2 bg-white shadow-md rounded-lg py-2 w-40" | ||
onMouseLeave={() => setIsUserDropdownOpen(false)} | ||
</div> | ||
</div> | ||
|
||
{/* 모바일 메뉴 */} | ||
{isMobileMenuOpen && ( | ||
<> | ||
<div | ||
className={`absolute left-0 w-full bg-white z-50 flex flex-col items-center p-6 shadow-lg transition-all duration-300 ease-in-out`} | ||
style={{ | ||
top: `${NAVBAR_HEIGHT}px`, | ||
transform: isMobileMenuOpen | ||
? "translateY(0)" | ||
: "translateY(-100%)", | ||
opacity: isMobileMenuOpen ? 1 : 0, | ||
}} | ||
> | ||
{mobileMenuItems.map((item) => ( | ||
<div key={item.label} className="w-full items-center"> | ||
{/* 메인 메뉴 버튼 */} | ||
<button | ||
onClick={() => toggleMenu(item.label)} | ||
className="text-lg font-semibold py-2 w-full" | ||
> | ||
{item.label} | ||
{openMenu === item.label ? ( | ||
<ExpandLess className="ml-2" /> | ||
) : ( | ||
<ExpandMore className="ml-2" /> | ||
)} | ||
</button> | ||
|
||
{openMenu === item.label && ( | ||
<div className="flex flex-col items-center w-full py-2"> | ||
{item.children.map((child) => ( | ||
<Link | ||
key={child.href} | ||
href={child.href} | ||
className="py-1 text-gray-700" | ||
> | ||
{child.label} | ||
</Link> | ||
))} | ||
</div> | ||
)} | ||
</div> | ||
))} | ||
|
||
{/* 대시보드 버튼 */} | ||
<Link | ||
href="https://easily-dashboard.jojicompany.com" | ||
className="bg-[#FF6B2B] text-white py-2 px-4 rounded-md hover:bg-[#e55a1f] mt-2" | ||
> | ||
<Link | ||
href="/myinfo" | ||
className="block px-4 py-2 text-sm text-gray-700 hover:bg-gray-100" | ||
> | ||
내 정보 | ||
</Link> | ||
대시보드 | ||
</Link> | ||
{isLoggedIn && ( | ||
<button | ||
onClick={handleLogout} | ||
className="block w-full text-left px-4 py-2 text-sm text-gray-700 hover:bg-gray-100" | ||
className="bg-red-500 text-white py-2 px-4 rounded-md hover:bg-red-600 mt-2" | ||
> | ||
로그아웃 | ||
</button> | ||
)} | ||
</div> | ||
</> | ||
)} | ||
</nav> | ||
|
||
{/* 오버레이 배경 */} | ||
{isMobileMenuOpen && ( | ||
<div | ||
className="fixed inset-0 bg-black opacity-30 z-40 transition-opacity duration-300 ease-in-out" | ||
onClick={() => setIsMobileMenuOpen(false)} | ||
></div> | ||
)} | ||
</> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
헤이 헤이 Chill 모바일 관련 네비게이션 UI가 꽤 잘 나뉘어 있네. static analysis 힌트 말대로 불필요한 Fragment가 한 군데 보이긴 해. 굳이 <> </>씬 안 써도 되니 살짝 정리하면 더 깔끔할 듯 해
- <>
<nav className="sticky bg-white top-0 z-50 w-full flex items-center shadow-sm">
...
</nav>
- </>
Committable suggestion skipped: line range outside the PR's diff.
- 아직 모바일 애니메이션 작동안함
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/app/_components/navBar.js (2)
19-20
: 상태 관리 개선 제안헤이 헤이 Chill... 모바일 메뉴 상태 관리가 잘 구현되어 있네요. 하지만 이걸 좀 더 chill하게 만들 수 있을 것 같아요.
- const [isMobileMenuOpen, setIsMobileMenuOpen] = useState(false); - const [openMenu, setOpenMenu] = useState(null); + const [mobileNav, setMobileNav] = useState({ + isMenuOpen: false, + activeMenu: null + });
246-251
: 오버레이 UX 개선헤이 헤이 Chill... 오버레이가 잘 구현되어 있네요. 하지만 사용자 경험을 좀 더 chill하게 만들 수 있을 것 같아요.
<div - className="fixed inset-0 bg-black opacity-30 z-40 transition-opacity duration-300 ease-in-out" + className="fixed inset-0 bg-black/30 backdrop-blur-sm z-40 transition-opacity duration-300 ease-in-out" onClick={() => setIsMobileMenuOpen(false)} + onKeyDown={(e) => e.key === 'Escape' && setIsMobileMenuOpen(false)} + role="presentation" ></div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json
(2 hunks)src/app/_components/navBar.js
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (1)
src/app/_components/navBar.js (1)
5-7
: 새로운 의존성 추가에 대한 검토헤이 헤이 Chill... 새로운 아이콘과 반응형 디자인을 위한 의존성이 잘 추가되었네요. 하지만 이런 의존성들이 번들 사이즈에 영향을 줄 수 있어서 한번 체크해볼 필요가 있어요.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check bundle size impact of new dependencies npm list react-icons @mui/icons-material react-responsive --json | jq '.dependencies | map_values(.version)' # Get package sizes npm view react-icons dist.size npm view @mui/icons-material dist.size npm view react-responsive dist.sizeLength of output: 280
헤이 헤이 Chill... 스크립트 결과가 기대한 출력을 주지 않았으니, 이번에는 package.json 파일을 직접 확인해서 의존성 버전과 npm 레지스트리에서 아이콘 및 반응형 관련 패키지의 사이즈 정보를 재확인하는 스크립트를 만들어 봤어요. 아래 스크립트를 실행해 확인 부탁드릴게요.
이 스크립트 결과를 통해 의존성 추가가 실제로 번들 사이즈에 미치는 영향을 직접 확인할 수 있으니, 실행 후 나온 결과를 바탕으로 최종 검토 부탁드려요.
🏁 Script executed:
#!/bin/bash # package.json 내에 추가된 의존성 버전 정보를 확인합니다. echo "package.json 에서 의존성 확인:" jq '{ "react-icons": .dependencies["react-icons"], "@mui/icons-material": .dependencies["@mui/icons-material"], "react-responsive": .dependencies["react-responsive"] }' package.json # npm 레지스트리에서 각 패키지의 dist.unpackedSize (패키지 사이즈 정보)을 조회합니다. echo "" echo "패키지 사이즈 (dist.unpackedSize) 확인:" echo "react-icons:" && npm view react-icons dist.unpackedSize || echo "데이터가 없습니다." echo "" echo "@mui/icons-material:" && npm view '@mui/icons-material' dist.unpackedSize || echo "데이터가 없습니다." echo "" echo "react-responsive:" && npm view react-responsive dist.unpackedSize || echo "데이터가 없습니다."Length of output: 739
헤이 헤이 Chill... 확인해본 결과 package.json에는 새 의존성들이 아래와 같이 등록되어 있고요,
• react-icons: ^5.4.0 (dist.unpackedSize: 86188055 바이트)
• @mui/icons-material: ^6.4.3 (dist.unpackedSize: 18014131 바이트)
• react-responsive: ^10.0.0 (dist.unpackedSize: 57482 바이트)를 확인할 수 있네요. react-responsive는 사이즈가 작아서 큰 문제는 없지만, react-icons와 @mui/icons-material의 압축 해제 사이즈가 꽤 큰 편이에요. 만약 최종 번들 사이즈가 중요한 이슈라면 lazy loading이나 코드 스플리팅 같은 최적화 기법을 적용해보는 것도 chill한 방법이 될 거예요.
src/app/_components/navBar.js
Outdated
const mobileMenuItems = [ | ||
{ | ||
label: "홈", | ||
baseUrl: "/", | ||
children: [ | ||
{ label: "홈", href: "/" }, | ||
{ label: "소개", href: "#hero" }, | ||
{ label: "서비스", href: "#service" }, | ||
{ label: "FAQ", href: "#faq" }, | ||
{ label: "구독", href: "#subscribe" }, | ||
{ label: "문의", href: "#contact" }, | ||
], | ||
}, | ||
{ | ||
label: "가격", | ||
baseUrl: "/price", | ||
children: [ | ||
{ label: "가격", href: "/price" }, | ||
{ label: "정책", href: "#policy" }, | ||
{ label: "결제", href: "#purchase" }, | ||
], | ||
}, | ||
{ | ||
label: "공지사항", | ||
baseUrl: "/post", | ||
children: [ | ||
{ label: "공지사항", href: "/post" }, | ||
{ label: "게시판", href: "#board" }, | ||
], | ||
}, | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
메뉴 구조 중복 개선 제안
헤이 헤이 Chill... 모바일용 메뉴 아이템이 데스크톱 메뉴와 많이 겹치네요. 이걸 좀 더 chill하게 관리할 수 있을 것 같아요.
-const mobileMenuItems = [...]
+const mobileMenuItems = menuItems.map(item => ({
+ ...item,
+ children: [
+ { label: item.label, href: item.baseUrl },
+ ...item.children
+ ]
+}));
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const mobileMenuItems = [ | |
{ | |
label: "홈", | |
baseUrl: "/", | |
children: [ | |
{ label: "홈", href: "/" }, | |
{ label: "소개", href: "#hero" }, | |
{ label: "서비스", href: "#service" }, | |
{ label: "FAQ", href: "#faq" }, | |
{ label: "구독", href: "#subscribe" }, | |
{ label: "문의", href: "#contact" }, | |
], | |
}, | |
{ | |
label: "가격", | |
baseUrl: "/price", | |
children: [ | |
{ label: "가격", href: "/price" }, | |
{ label: "정책", href: "#policy" }, | |
{ label: "결제", href: "#purchase" }, | |
], | |
}, | |
{ | |
label: "공지사항", | |
baseUrl: "/post", | |
children: [ | |
{ label: "공지사항", href: "/post" }, | |
{ label: "게시판", href: "#board" }, | |
], | |
}, | |
]; | |
const mobileMenuItems = menuItems.map(item => ({ | |
...item, | |
children: [ | |
{ label: item.label, href: item.baseUrl }, | |
...item.children | |
] | |
})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개선되면 좋겠는 항목 남겨두었습니다!! 작업하시느라 고생하셨어요.
* feat: 서비스섹션 구현(기본구조) * feat: 폰트크기 변경 * feat: 서비스섹션 ui개선 -생동감이 느껴지도록 하였음 * feat: 서비스섹션 ui 수정 * feat: 공지사항 페이지 ui 수정 -표시되는 시간을 YYYY-MM-DD형식으로 수정 * feat: 모바일 네비게이션 바 수정 -애니메이션이 정상 작동하도록 수정 * feat: 네비게이션 바 경로수정 * feat: contact 삭제 * feat: contact 삭제 * feat: 메뉴아이템 추출 -serviceItem.js 로 컴포넌트분리 * feat: 시간형식 변환 함수 모듈화 -useFormatDate * feat: 포맷데이트 유틸화 - _utils/formatDate.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/_components/navBar.js (1)
186-242
: 🛠️ Refactor suggestion모바일 메뉴 접근성 개선
헤이 헤이 Chill... 모바일 메뉴가 chill하게 동작하고 있지만, 키보드 사용자와 스크린 리더 사용자를 위한 접근성이 필요해요.
<div className={`absolute left-0 w-full bg-white z-40 flex flex-col items-center p-6 shadow-lg duration-300 ease-in-out ${ isMobileMenuOpen ? "opacity-100 translate-y-0" : "opacity-0 -translate-y-full" }`} style={{ top: `${NAVBAR_HEIGHT}px` }} + role="menu" + aria-label="모바일 메뉴" + aria-hidden={!isMobileMenuOpen} > {mobileMenuItems.map((item) => ( <div key={item.label} className="w-full items-center"> <button onClick={() => toggleMenu(item.label)} className="text-lg font-semibold py-2 w-full" + aria-expanded={openMenu === item.label} + aria-controls={`${item.label}-submenu`} > {item.label} {openMenu === item.label ? ( <ExpandLess className="ml-2" /> ) : ( <ExpandMore className="ml-2" /> )} </button> {openMenu === item.label && ( - <div className="flex flex-col items-center w-full py-2"> + <div + id={`${item.label}-submenu`} + role="menu" + className="flex flex-col items-center w-full py-2" + > {item.children.map((child) => ( <Link key={child.href} href={child.href} className="py-1 text-gray-700" + role="menuitem" > {child.label} </Link> ))} </div> )} </div> ))}
🧹 Nitpick comments (5)
src/app/_utils/formatDate.js (1)
1-4
: 날짜 포맷팅 함수에 에러 처리가 필요해요헤이 헤이 Chill... 날짜 포맷팅 함수가 깔끔하네요. 하지만 잘못된 날짜 문자열이 들어오면 에러가 날 수 있어서 조금 걱정이에요. chill chill하게 에러 처리를 추가해보는게 어떨까요?
export const formatDate = (dateString) => { + if (!dateString) return ''; const date = new Date(dateString); + if (isNaN(date.getTime())) return ''; return date.toISOString().split("T")[0]; };src/app/page.js (1)
4-4
: 사용하지 않는 import문이 있어요헤이 헤이 Chill... ContactSection을 import 했는데 사용하지 않고 있네요. chill하게 제거해주면 좋을 것 같아요.
-import { ContactSection } from "./_components/sections/contactSection";
src/app/post/page.js (2)
34-46
: 에러 처리를 개선해보면 좋을 것 같아요헤이 헤이 Chill... fetchPosts 함수에서 에러가 발생했을 때 사용자에게 알려주면 좋을 것 같아요. chill하게 try-catch를 추가해보는건 어떨까요?
const fetchPosts = async (page) => { setLoading(true); + try { const response = await fetch( `${process.env.NEXT_PUBLIC_BASE_URL}/posts?page=${page}&limit=6&categories=NOTICE` ); if (!response.ok) { throw new Error(`HTTP error! status: ${response.status}`); } const notice = await response.json(); setPosts(notice.data); setTotalPages(notice.meta.totalPages); + } catch (error) { + console.error('게시물을 불러오는 중 에러가 발생했습니다:', error); + setPosts([]); + setTotalPages(1); + } setLoading(false); };
52-60
: 로딩 상태 UI를 개선해보면 좋을 것 같아요헤이 헤이 Chill... 로딩 중일 때 보여주는 UI가 조금 단순해 보여요. chill chill하게 실제 카드 레이아웃과 비슷한 스켈레톤을 보여주면 어떨까요?
if (loading) { return ( - <div className="flex flex-col gap-4 mt-10"> - <Skeleton className="h-8 w-full mb-4" /> - <Skeleton className="h-6 w-5/6 mb-2" /> - <Skeleton className="h-6 w-3/4" /> + <div className="bg-white rounded-lg shadow-md p-6 max-w-5xl mx-auto mt-8"> + <Skeleton className="h-8 w-48 mx-auto mb-6" /> + <div className="space-y-4"> + {[1, 2, 3].map((i) => ( + <div key={i} className="border rounded-lg p-4"> + <div className="flex justify-between items-center"> + <div className="space-y-2"> + <Skeleton className="h-6 w-64" /> + <Skeleton className="h-4 w-48" /> + <Skeleton className="h-4 w-32" /> + </div> + <Skeleton className="h-4 w-24" /> + </div> + </div> + ))} + </div> </div> ); }src/app/_components/sections/serviceSection.js (1)
15-22
: 비디오 성능 최적화 제안헤이 헤이 Chill... 비디오 요소가 chill하게 동작하고 있지만, 성능을 더 개선할 수 있을 것 같아요.
<video className="w-full h-full object-cover" src={item.videoSrc} autoPlay loop muted playsInline + loading="lazy" + preload="none" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/_components/navBar.js
(3 hunks)src/app/_components/sections/serviceSection.js
(1 hunks)src/app/_consts/serviceItem.js
(1 hunks)src/app/_utils/formatDate.js
(1 hunks)src/app/page.js
(1 hunks)src/app/post/page.js
(2 hunks)src/app/postDetail/[id]/page.js
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/postDetail/[id]/page.js
🔇 Additional comments (6)
src/app/page.js (1)
26-31
: 섹션 높이 설정을 검토해주세요헤이 헤이 Chill... Review와 FAQ 섹션의 높이가 h-1/2로 설정되어 있는데, 화면 크기에 따라 콘텐츠가 잘릴 수 있어요. chill chill하게 min-height를 사용하는 건 어떨까요?
<section id="review" - className="h-1/2 flex items-center justify-center bg-gray-100" + className="min-h-[50vh] flex items-center justify-center bg-gray-100" > <ReviewSection /> </section> {/* FAQ Section */} <section id="faq" - className="h-1/2 flex items-center justify-center bg-gray-100" + className="min-h-[50vh] flex items-center justify-center bg-gray-100" >Also applies to: 33-38
src/app/_consts/serviceItem.js (1)
4-5
: 서비스 설명 텍스트 업데이트 필요헤이 헤이 Chill... 서비스 설명이 모두 동일한 placeholder 텍스트를 사용하고 있네요. 각 서비스의 특징을 잘 표현하도록 수정하면 더 chill할 것 같아요.
실제 서비스 설명으로 업데이트가 필요합니다. 각 서비스의 특징과 장점을 잘 설명하는 텍스트로 변경해주세요.
Also applies to: 11-12, 18-19
src/app/_components/navBar.js (4)
5-7
: 아이콘 라이브러리 변경 필요헤이 헤이 Chill... 이전 리뷰에서 말씀드린 것처럼 lucide-react를 사용하면 더 chill할 것 같아요.
136-383
: 컴포넌트 분리 필요헤이 헤이 Chill... 네비게이션 코드가 너무 복잡해졌네요. mobile과 desktop 컴포넌트로 분리하면 더 chill하게 관리할 수 있을 거예요.
100-129
: 메뉴 구조 중복 개선헤이 헤이 Chill... 모바일 메뉴 아이템이 데스크톱 메뉴와 많이 겹치네요. 이걸 좀 더 chill하게 관리하면 좋을 것 같아요.
270-270
: 불필요한 조건 체크 제거헤이 헤이 Chill...
!isMobile
체크가 중복되어 있네요. 이미 상위에서 체크하고 있어서 이건 좀 chill하게 정리해도 될 것 같아요.Also applies to: 380-380
videoSrc: | ||
"http://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerBlazes.mp4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP URL을 HTTPS로 변경 필요
헤이 헤이 Chill... 비디오 URL이 HTTP를 사용하고 있는데 이건 좀 chill하지 않네요. 브라우저에서 mixed content 경고가 발생할 수 있어요.
- "http://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerBlazes.mp4",
+ "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerBlazes.mp4",
Also applies to: 13-14, 20-21
key={item.id} | ||
className="relative bg-opacity-90 rounded-2xl shadow-sm p-10 mb-16 max-w-6xl mx-auto transition-transform duration-300 hover:scale-105" | ||
> | ||
<h2 className="text-4xl font-bold mb-4">{item.title}</h2> | ||
<p className="text-lg text-black mb-6">{item.subtitle}</p> | ||
<div className="relative w-full aspect-video overflow-hidden rounded-lg shadow-lg"> | ||
<video | ||
className="w-full h-full object-cover" | ||
src={item.videoSrc} | ||
autoPlay | ||
loop | ||
muted | ||
playsInline | ||
/> | ||
</div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
접근성 개선 필요
헤이 헤이 Chill... 서비스 아이템들이 chill하게 보이긴 하지만, 스크린 리더 사용자들을 위한 접근성이 부족해요.
<div
key={item.id}
className="relative bg-opacity-90 rounded-2xl shadow-sm p-10 mb-16 max-w-6xl mx-auto transition-transform duration-300 hover:scale-105"
+ role="article"
+ aria-labelledby={`service-title-${item.id}`}
>
- <h2 className="text-4xl font-bold mb-4">{item.title}</h2>
+ <h2 id={`service-title-${item.id}`} className="text-4xl font-bold mb-4">{item.title}</h2>
<p className="text-lg text-black mb-6">{item.subtitle}</p>
<div className="relative w-full aspect-video overflow-hidden rounded-lg shadow-lg">
<video
className="w-full h-full object-cover"
src={item.videoSrc}
autoPlay
loop
muted
playsInline
+ aria-label={`${item.title} 서비스 소개 영상`}
/>
</div>
</div>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div | |
key={item.id} | |
className="relative bg-opacity-90 rounded-2xl shadow-sm p-10 mb-16 max-w-6xl mx-auto transition-transform duration-300 hover:scale-105" | |
> | |
<h2 className="text-4xl font-bold mb-4">{item.title}</h2> | |
<p className="text-lg text-black mb-6">{item.subtitle}</p> | |
<div className="relative w-full aspect-video overflow-hidden rounded-lg shadow-lg"> | |
<video | |
className="w-full h-full object-cover" | |
src={item.videoSrc} | |
autoPlay | |
loop | |
muted | |
playsInline | |
/> | |
</div> | |
</div> | |
<div | |
key={item.id} | |
className="relative bg-opacity-90 rounded-2xl shadow-sm p-10 mb-16 max-w-6xl mx-auto transition-transform duration-300 hover:scale-105" | |
role="article" | |
aria-labelledby={`service-title-${item.id}`} | |
> | |
<h2 id={`service-title-${item.id}`} className="text-4xl font-bold mb-4">{item.title}</h2> | |
<p className="text-lg text-black mb-6">{item.subtitle}</p> | |
<div className="relative w-full aspect-video overflow-hidden rounded-lg shadow-lg"> | |
<video | |
className="w-full h-full object-cover" | |
src={item.videoSrc} | |
autoPlay | |
loop | |
muted | |
playsInline | |
aria-label={`${item.title} 서비스 소개 영상`} | |
/> | |
</div> | |
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/app/_components/navBar.js (2)
99-128
: 🛠️ Refactor suggestion메뉴 구조 중복 제거
헤이 헤이 Chill... 모바일이랑 데스크톱 메뉴 구조가 거의 비슷한데 중복이 있네요. 좀 더 chill하게 정리해 보죠.
-const mobileMenuItems = [ - { - label: "홈", - baseUrl: "/", - children: [ - { label: "홈", href: "/" }, - { label: "소개", href: "#hero" }, - ... - ], - }, - ... -]; +const mobileMenuItems = menuItems.map(item => ({ + ...item, + children: [ + { label: item.label, href: item.baseUrl }, + ...item.children + ] +}));
135-135
: 🛠️ Refactor suggestion컴포넌트 분리 필요
헤이 헤이 Chill... 이전 리뷰 코멘트에서도 언급됐듯이 NavBar JSX가 너무 복잡해졌네요. 모바일이랑 데스크톱 컴포넌트를 분리하면 좀 더 chill하게 관리할 수 있을 것 같아요.
+const MobileNavBar = ({ isLoggedIn, userName, handleLogout, ... }) => { + // 모바일 관련 로직 +}; +const DesktopNavBar = ({ isLoggedIn, userName, handleLogout, ... }) => { + // 데스크톱 관련 로직 +}; export default function NavBar() { // ... states and effects return ( <> - {isMobile && ( - // ... mobile JSX - )} - {!isMobile && ( - // ... desktop JSX - )} + {isMobile ? <MobileNavBar {...props} /> : <DesktopNavBar {...props} />} </> ); }
🧹 Nitpick comments (1)
src/app/_components/navBar.js (1)
185-241
: 모바일 메뉴 접근성 개선헤이 헤이 Chill... 모바일 메뉴가 잘 동작하긴 하는데, 키보드 사용자나 스크린 리더 사용자를 위한 접근성이 좀 부족해 보이네요. 좀 더 chill하게 개선해 보죠.
<div className={`absolute left-0 w-full bg-white z-40 flex flex-col items-center p-6 shadow-lg`} + role="menu" + aria-label="모바일 메뉴" style={{ top: `${NAVBAR_HEIGHT}px` }} > {mobileMenuItems.map((item) => ( <div key={item.label} className="w-full items-center"> <button onClick={() => toggleMenu(item.label)} className="text-lg font-semibold py-2 w-full flex justify-center items-center" + aria-expanded={openMenu === item.label} + aria-controls={`${item.label}-submenu`} >
src/app/_components/navBar.js
Outdated
padding: isMenuDropdownOpen ? "1.25rem" : "0px", | ||
}} | ||
></div> | ||
{!isMobile && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
불필요한 조건 체크 제거
헤이 헤이 Chill... 데스크톱 뷰에서 !isMobile
체크가 중복되어 있네요. 이미 상위에서 체크하고 있어서 이건 좀 chill하게 정리해도 될 것 같아요.
-{!isMobile && (
<div className="flex items-center justify-between px-6 w-full">
...
</div>
-)}
Also applies to: 379-379
src/app/_components/navBar.js
Outdated
{openMenu === item.label ? ( | ||
<ChevronDown className="ml-2" /> | ||
) : ( | ||
<ChevronUp className="ml-2" /> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아이콘 방향 오류
헤이 헤이 Chill... 메뉴가 열렸을 때와 닫혔을 때 아이콘이 반대로 되어있네요. 좀 더 직관적으로 수정해 보죠.
-{openMenu === item.label ? (
- <ChevronDown className="ml-2" />
-) : (
- <ChevronUp className="ml-2" />
-)}
+{openMenu === item.label ? (
+ <ChevronUp className="ml-2" />
+) : (
+ <ChevronDown className="ml-2" />
+)}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{openMenu === item.label ? ( | |
<ChevronDown className="ml-2" /> | |
) : ( | |
<ChevronUp className="ml-2" /> | |
)} | |
{openMenu === item.label ? ( | |
<ChevronUp className="ml-2" /> | |
) : ( | |
<ChevronDown className="ml-2" /> | |
)} |
src/app/_components/navBar.js
Outdated
import { useMediaQuery } from "react-responsive"; | ||
import { ChevronDown, ChevronUp, TableOfContents, X } from "lucide-react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
불필요한 의존성 제거 필요
헤이 헤이 Chill... 이전 리뷰 코멘트에서 언급된 것처럼 lucide-react를 사용하라고 했는데, 여전히 react-responsive를 사용하고 있네요. 좀 더 chill한 방법으로 해결해 보죠.
-import { useMediaQuery } from "react-responsive";
+const useIsMobile = () => {
+ const [isMobile, setIsMobile] = useState(false);
+
+ useEffect(() => {
+ const checkMobile = () => {
+ setIsMobile(window.innerWidth <= 768);
+ };
+
+ checkMobile();
+ window.addEventListener('resize', checkMobile);
+ return () => window.removeEventListener('resize', checkMobile);
+ }, []);
+
+ return isMobile;
+};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { useMediaQuery } from "react-responsive"; | |
import { ChevronDown, ChevronUp, TableOfContents, X } from "lucide-react"; | |
const useIsMobile = () => { | |
const [isMobile, setIsMobile] = useState(false); | |
useEffect(() => { | |
const checkMobile = () => { | |
setIsMobile(window.innerWidth <= 768); | |
}; | |
checkMobile(); | |
window.addEventListener('resize', checkMobile); | |
return () => window.removeEventListener('resize', checkMobile); | |
}, []); | |
return isMobile; | |
}; | |
import { ChevronDown, ChevronUp, TableOfContents, X } from "lucide-react"; |
-mobileNavBar -desktopNavBar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/app/_components/mobileNavBar.js (1)
163-167
: 아이콘 방향 수정 필요헤이 헤이 Chill... 메뉴가 열렸을 때와 닫혔을 때 아이콘이 반대로 되어있네요. 좀 더 직관적으로 수정해 보죠.
- {openMenu === item.label ? ( - <ChevronDown className="ml-2" /> - ) : ( - <ChevronUp className="ml-2" /> - )} + {openMenu === item.label ? ( + <ChevronUp className="ml-2" /> + ) : ( + <ChevronDown className="ml-2" /> + )}src/app/_components/desktopNavBar.js (2)
98-105
: 불필요한 빈 div 제거헤이 헤이 Chill... 여기 빈 div가 있네요. 스타일만 있고 내용이 없어요. 이런 건 좀 더 chill하게 정리해 보죠.
- <div - className={`fixed left-0 bg-white shadow-md rounded-b-lg p-5 w-full - transition-all duration-300 ease-in-out`} - style={{ - top: `${NAVBAR_HEIGHT}px`, - height: isMenuDropdownOpen ? `${DROPDOWN_BAR_HEIGHT}px` : "0px", - padding: isMenuDropdownOpen ? "1.25rem" : "0px", - }} - ></div>
206-207
: 불필요한 console.log 제거헤이 헤이 Chill... 프로덕션 코드에 console.log가 남아있네요. 이런 건 배포 전에 정리하는 게 좋죠.
onClick={() => { - console.log("로그인 버튼 클릭됨"); redirectToLogin(); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/_components/desktopNavBar.js
(1 hunks)src/app/_components/mobileNavBar.js
(1 hunks)src/app/_components/navBar.js
(1 hunks)
import { useMediaQuery } from "react-responsive"; | ||
import MobileNavbar from "./mobileNavBar"; | ||
import DesktopNavbar from "./desktopNavBar"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
불필요한 의존성 제거 필요
헤이 헤이 Chill... react-responsive 라이브러리가 아직도 있네요. 좀 더 chill한 방법으로 해결해 보죠.
-import { useMediaQuery } from "react-responsive";
+import { useState, useEffect } from "react";
import MobileNavbar from "./mobileNavBar";
import DesktopNavbar from "./desktopNavBar";
+
+const useIsMobile = () => {
+ const [isMobile, setIsMobile] = useState(false);
+
+ useEffect(() => {
+ const checkMobile = () => {
+ setIsMobile(window.innerWidth <= 768);
+ };
+
+ checkMobile();
+ window.addEventListener('resize', checkMobile);
+ return () => window.removeEventListener('resize', checkMobile);
+ }, []);
+
+ return isMobile;
+};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { useMediaQuery } from "react-responsive"; | |
import MobileNavbar from "./mobileNavBar"; | |
import DesktopNavbar from "./desktopNavBar"; | |
import { useState, useEffect } from "react"; | |
import MobileNavbar from "./mobileNavBar"; | |
import DesktopNavbar from "./desktopNavBar"; | |
const useIsMobile = () => { | |
const [isMobile, setIsMobile] = useState(false); | |
useEffect(() => { | |
const checkMobile = () => { | |
setIsMobile(window.innerWidth <= 768); | |
}; | |
checkMobile(); | |
window.addEventListener('resize', checkMobile); | |
return () => window.removeEventListener('resize', checkMobile); | |
}, []); | |
return isMobile; | |
}; |
<div | ||
className={`absolute left-0 w-full bg-white z-40 flex flex-col items-center p-6 shadow-lg | ||
duration-300 ease-in-out ${ | ||
isMobileMenuOpen | ||
? "opacity-100 translate-y-0" | ||
: "opacity-0 -translate-y-full" | ||
}`} | ||
style={{ top: `${NAVBAR_HEIGHT}px` }} | ||
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
모바일 메뉴 접근성 개선 필요
헤이 헤이 Chill... 모바일 메뉴가 잘 구현되어 있지만, 키보드 접근성이나 스크린 리더 지원이 좀 부족해 보여요.
<div
className={`absolute left-0 w-full bg-white z-40 flex flex-col items-center p-6 shadow-lg
duration-300 ease-in-out ${
isMobileMenuOpen
? "opacity-100 translate-y-0"
: "opacity-0 -translate-y-full"
}`}
+ role="menu"
+ aria-label="모바일 메뉴"
style={{ top: `${NAVBAR_HEIGHT}px` }}
>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div | |
className={`absolute left-0 w-full bg-white z-40 flex flex-col items-center p-6 shadow-lg | |
duration-300 ease-in-out ${ | |
isMobileMenuOpen | |
? "opacity-100 translate-y-0" | |
: "opacity-0 -translate-y-full" | |
}`} | |
style={{ top: `${NAVBAR_HEIGHT}px` }} | |
> | |
<div | |
className={`absolute left-0 w-full bg-white z-40 flex flex-col items-center p-6 shadow-lg | |
duration-300 ease-in-out ${ | |
isMobileMenuOpen | |
? "opacity-100 translate-y-0" | |
: "opacity-0 -translate-y-full" | |
}`} | |
role="menu" | |
aria-label="모바일 메뉴" | |
style={{ top: `${NAVBAR_HEIGHT}px` }} | |
> |
-useAuth 커스텀훅으로 구현
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/app/_components/hooks/useAuth.js (2)
8-34
: 헤이 헤이 Chill... 에러 처리를 좀 더 여유롭게 해보면 어떨까요에러 처리가 기본적으로는 잘 되어있지만, 네트워크 문제나 서버 장애같은 상황도 chill하게 대처할 수 있게 해주면 좋을 것 같아요.
이렇게 수정해보는건 어떠세요:
.catch((error) => { if (error.message === "Unauthorized") { setIsLoggedIn(false); + } else { + console.error("서버 연결에 문제가 있어요. chill하게 기다려주세요.", error); + // 재시도 로직 추가 + setTimeout(() => { + // 재시도 로직 + }, 5000); } })
36-53
: 헤이 헤이 Chill... 로그아웃할 때도 사용자에게 상태를 알려주면 좋을 것 같아요로그아웃 처리가 잘 되어있는데, 사용자가 더 chill한 경험을 할 수 있도록 로딩 상태랑 에러 처리를 추가해보면 어떨까요.
이런 식으로 수정해보는건 어떠세요:
const handleLogout = () => { + setIsLoading(true); fetch(`${process.env.NEXT_PUBLIC_BASE_URL}/auth/logout`, { method: "POST", credentials: "include", }).then((response) => { if (response.status === 201) { setIsLoggedIn(false); } else { - console.error("Failed to logout."); + console.error("로그아웃에 실패했어요. chill하게 다시 시도해보세요."); } + }).catch((error) => { + console.error("서버와 통신 중 문제가 발생했어요. 진정하고 다시 시도해보세요.", error); + }).finally(() => { + setIsLoading(false); }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/_components/desktopNavBar.js
(1 hunks)src/app/_components/hooks/useAuth.js
(1 hunks)src/app/_components/mobileNavBar.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/_components/desktopNavBar.js
- src/app/_components/mobileNavBar.js
🔇 Additional comments (3)
src/app/_components/hooks/useAuth.js (3)
1-7
: 헤이 헤이 Chill... 상태 관리가 깔끔하네요상태 변수들이 잘 정의되어 있고 한글 주석도 chill하게 잘 달아주셨어요. 이렇게 명확한 코드는 보기만 해도 마음이 평화로워져요.
55-62
: 헤이 헤이 Chill... 깔끔한 반환값이네요필요한 상태와 함수들이 chill chill하게 잘 정리되어 있어요. 이렇게 깔끔한 코드를 보니 마음이 평화로워지네요.
64-64
: 헤이 헤이 Chill... 깔끔한 내보내기네요default export로 chill하게 잘 내보내주셨어요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
크 모듈화의 황제가 되어가고 있는 모습이 보이는 군요.
단 한가지 문제점이 있어서, 한 단계 더 레벨업 시켜드리겠습니다
src/app/_components/navBar.js
Outdated
import { FaBars, FaTimes } from "react-icons/fa"; | ||
import { useMediaQuery } from "react-responsive"; | ||
import { ExpandMore, ExpandLess } from "@mui/icons-material"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
package.json에서 react-icons/fa, @mui/icons-material 패키지를 제거하고, 다시 npm i 해서 커밋해주세용
src/app/_components/desktopNavBar.js
Outdated
const { isLoggedIn, isLoading, userName, redirectToLogin, handleLogout } = | ||
useAuth(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useAuth hook의 상태들은 지역스코프로 관리되는 상태이기 때문에 DesktopNavbar <-> MobileNavbar로 전환될때마다 매번 호출이 일어날 가능성이 있어요. 이 둘이 같은 상태를 공유하도록 하는 작업이 필요합니다. 요건 따로 설명드릴게요
-props로 같은상태 공유
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/_components/desktopNavBar.js (1)
131-169
: 🛠️ Refactor suggestion인증 상태 처리 개선 필요
헤이 헤이 Chill... 인증 상태 처리가 좀 복잡해 보이네요. 이전 리뷰 코멘트에서 언급된 것처럼, 모바일이랑 데스크톱 간에 상태가 공유되도록 chill하게 개선이 필요해요.
-{isLoading ? null : isLoggedIn ? ( +const AuthSection = () => { + if (isLoading) return null; + + return isLoggedIn ? ( + <UserMenu userName={userName} onLogout={handleLogout} /> + ) : ( + <LoginButton onLogin={redirectToLogin} /> + ); +};
🧹 Nitpick comments (3)
src/app/_components/navBar.js (1)
12-17
: 불필요한 Fragment 제거헤이 헤이 Chill... Fragment가 굳이 필요하지 않아 보이네요. 코드를 좀 더 chill하게 정리해 보죠.
-<> - {isMobile && <MobileNavbar {...auth} />} - {/*데스크톱*/} - {!isMobile && <DesktopNavbar {...auth} />} -</> +{isMobile ? <MobileNavbar {...auth} /> : <DesktopNavbar {...auth} />}src/app/_components/desktopNavBar.js (2)
17-18
: 상태 관리 최적화 필요헤이 헤이 Chill... 드롭다운 상태들이 각각 따로 관리되고 있네요. 이걸 하나의 상태로 chill하게 관리하면 어떨까요.
-const [isMenuDropdownOpen, setIsMenuDropdownOpen] = useState(false); -const [isUserDropdownOpen, setIsUserDropdownOpen] = useState(false); +const [openDropdown, setOpenDropdown] = useState(null); // 'menu' | 'user' | null
20-47
: 메뉴 구조 최적화 제안헤이 헤이 Chill... 메뉴 구조가 좀 하드코딩되어 있네요. 이걸 설정 파일로 분리하면 관리하기가 더 chill할 것 같아요.
+// src/config/navigation.js +export const menuItems = [ + { + label: "홈", + baseUrl: "/", + children: [ + { label: "소개", href: "#hero" }, + // ... + ], + }, + // ... +];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/_components/desktopNavBar.js
(1 hunks)src/app/_components/mobileNavBar.js
(1 hunks)src/app/_components/navBar.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/_components/mobileNavBar.js
🔇 Additional comments (1)
src/app/_components/navBar.js (1)
2-2
: react-responsive 라이브러리 제거 필요헤이 헤이 Chill... 이전 리뷰에서 react-responsive를 제거하기로 했었는데, 아직도 남아있네요. 좀 더 chill한 방법으로 해결해 보죠.
-import { useMediaQuery } from "react-responsive"; +import { useState, useEffect } from "react"; + +const useIsMobile = () => { + const [isMobile, setIsMobile] = useState(false); + + useEffect(() => { + const checkMobile = () => { + setIsMobile(window.innerWidth <= 768); + }; + + checkMobile(); + window.addEventListener('resize', checkMobile); + return () => window.removeEventListener('resize', checkMobile); + }, []); + + return isMobile; +};
src/app/_components/desktopNavBar.js
Outdated
onClick={() => { | ||
console.log("로그인 버튼 클릭됨"); | ||
redirectToLogin(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
console.log 제거 필요
헤이 헤이 Chill... 프로덕션 코드에 console.log가 남아있네요. 이런 건 개발할 때만 쓰고 배포 전에는 chill하게 정리하는 게 좋죠.
onClick={() => {
- console.log("로그인 버튼 클릭됨");
redirectToLogin();
}}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
onClick={() => { | |
console.log("로그인 버튼 클릭됨"); | |
redirectToLogin(); | |
onClick={() => { | |
redirectToLogin(); | |
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아따 믓집니다잉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/app/_components/navBar.js (1)
2-2
: 🛠️ Refactor suggestionreact-responsive 라이브러리 제거 필요
헤이 헤이 Chill... 이전 리뷰에서 말씀드린 것처럼 react-responsive를 사용하지 않고 더 chill한 방법으로 해결해 보죠.
-import { useMediaQuery } from "react-responsive"; +import { useState, useEffect } from "react"; + +const useIsMobile = () => { + const [isMobile, setIsMobile] = useState(false); + + useEffect(() => { + const checkMobile = () => { + setIsMobile(window.innerWidth <= 768); + }; + + checkMobile(); + window.addEventListener('resize', checkMobile); + return () => window.removeEventListener('resize', checkMobile); + }, []); + + return isMobile; +}; -const isMobile = useMediaQuery({ maxWidth: 768 }); +const isMobile = useIsMobile();Also applies to: 8-8
src/app/_components/mobileNavBar.js (2)
119-123
:⚠️ Potential issue아이콘 방향 수정 필요
헤이 헤이 Chill... 메뉴가 열렸을 때와 닫혔을 때 아이콘이 반대로 되어있네요. 좀 더 chill하게 수정해 보죠.
- {openMenu === item.label ? ( - <ChevronDown className="ml-2" /> - ) : ( - <ChevronUp className="ml-2" /> - )} + {openMenu === item.label ? ( + <ChevronUp className="ml-2" /> + ) : ( + <ChevronDown className="ml-2" /> + )}
102-110
: 🛠️ Refactor suggestion모바일 메뉴 접근성 개선 필요
헤이 헤이 Chill... 모바일 메뉴가 잘 구현되어 있지만, 키보드 접근성이나 스크린 리더 지원이 좀 부족해 보여요.
<div className={`absolute left-0 w-full bg-white z-40 flex flex-col items-center p-6 shadow-lg duration-300 ease-in-out ${ isMobileMenuOpen ? "opacity-100 translate-y-0" : "opacity-0 -translate-y-full" }`} + role="menu" + aria-label="모바일 메뉴" style={{ top: `${NAVBAR_HEIGHT}px` }} >
🧹 Nitpick comments (3)
src/app/_components/mobileNavBar.js (1)
93-93
: 불필요한 테스트 클래스 제거헤이 헤이 Chill... 테스트용 클래스가 남아있네요. 프로덕션 코드는 chill하게 깔끔하게 가져가죠.
- className="text-2xl test" + className="text-2xl"src/app/_components/desktopNavBar.js (2)
111-111
: URL 구성 방식 개선 필요헤이 헤이 Chill... URL을 문자열로 직접 연결하는 건 좀 위험할 수 있어요. 더 chill한 방법으로 개선해 보죠.
- href={`${menu.baseUrl}${child.href}`} + href={new URL(child.href, menu.baseUrl).toString()}
124-124
: 색상값 CSS 변수로 관리 제안헤이 헤이 Chill... 하드코딩된 색상값들이 보이네요. 이걸 CSS 변수로 관리하면 더 chill하게 유지보수할 수 있을 것 같아요.
- className="bg-[#FF6B2B] text-white py-2 px-4 rounded-md hover:bg-[#e55a1f]" + className="bg-primary text-white py-2 px-4 rounded-md hover:bg-primary-dark"tailwind.config.js에 다음과 같이 추가해주세요:
module.exports = { theme: { extend: { colors: { primary: '#FF6B2B', 'primary-dark': '#e55a1f', }, }, }, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/_components/desktopNavBar.js
(1 hunks)src/app/_components/mobileNavBar.js
(1 hunks)src/app/_components/navBar.js
(1 hunks)
🔇 Additional comments (1)
src/app/_components/desktopNavBar.js (1)
163-163
: console.log 제거 필요헤이 헤이 Chill... 프로덕션 코드에 console.log가 남아있네요. 이런 건 개발할 때만 쓰고 배포 전에는 chill하게 정리하는 게 좋죠.
- console.log("로그인 버튼 클릭됨");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/page.js
(1 hunks)
🔇 Additional comments (2)
src/app/page.js (2)
25-30
: 리뷰 섹션의 반응형 디자인 확인이 필요해요헤이 헤이 Chill... 리뷰 섹션의 높이를 h-1/2로 변경한 게 보이네요. 콘텐츠가 많아지면 화면이 좀 불편해질 수 있을 것 같아요. min-h-screen이나 min-h-[50vh]를 사용하는 게 어떨까요? chill chill하게 생각해보면 콘텐츠가 잘리는 것보다는 여유있게 보여주는 게 좋을 것 같네요.
모바일에서 콘텐츠가 잘리지 않는지 확인해보시는 게 좋을 것 같아요.
9-46
: 연락처 섹션이 사라진 것 같아요헤이 헤이 Chill... ContactSection이 완전히 사라진 걸 확인했어요. 사용자들이 연락할 방법이 다른 곳에 있는지 chill하게 확인해보면 좋을 것 같아요. 혹시 SubscribeSection에서 처리하고 있나요?
사용자들이 회사에 연락할 수 있는 대체 방법이 있는지 확인해주세요. 완전 chill하게 처리하려면 푸터나 다른 섹션에 연락처 정보를 넣는 것도 좋을 것 같아요.
src/app/page.js
Outdated
<section | ||
id="faq" | ||
className="h-1/2 flex items-center justify-center bg-gray-100" | ||
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
FAQ 섹션도 같은 이슈가 있어요
헤이 헤이 Chill... FAQ 섹션도 리뷰 섹션이랑 같은 스타일로 가고 있네요. 디자인의 일관성은 좋지만 FAQ는 보통 콘텐츠가 더 많은 편이라 h-1/2가 좀 빡빡할 수 있어요. chill하게 min-height를 주면 콘텐츠 길이에 따라 자연스럽게 늘어날 수 있을 거예요.
- className="h-1/2 flex items-center justify-center bg-gray-100"
+ className="min-h-[50vh] flex items-center justify-center bg-gray-100"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<section | |
id="faq" | |
className="h-1/2 flex items-center justify-center bg-gray-100" | |
> | |
<section | |
id="faq" | |
className="min-h-[50vh] flex items-center justify-center bg-gray-100" | |
> |
* feat: 소개세션 반응형 디자인 * feat: 모바일 네비게이션 구현 * feat: 모바일 네비게이션바 개선 * feat: 중첩되는 코드 수정 * feat: 코드 재정렬 - 아직 모바일 애니메이션 작동안함 * [JOJI-775] 서비스섹션 (#35) * feat: 서비스섹션 구현(기본구조) * feat: 폰트크기 변경 * feat: 서비스섹션 ui개선 -생동감이 느껴지도록 하였음 * feat: 서비스섹션 ui 수정 * feat: 공지사항 페이지 ui 수정 -표시되는 시간을 YYYY-MM-DD형식으로 수정 * feat: 모바일 네비게이션 바 수정 -애니메이션이 정상 작동하도록 수정 * feat: 네비게이션 바 경로수정 * feat: contact 삭제 * feat: contact 삭제 * feat: 메뉴아이템 추출 -serviceItem.js 로 컴포넌트분리 * feat: 시간형식 변환 함수 모듈화 -useFormatDate * feat: 포맷데이트 유틸화 - _utils/formatDate.js * feat: 네비바 아이콘 통일 * feat: nav바 모바일/데스크톱 컴포넌트 분리 -mobileNavBar -desktopNavBar * feat: 로그인 기능 모듈화 -useAuth 커스텀훅으로 구현 * feat: 네비바 상태공유 -props로 같은상태 공유 * feat: props 컨벤션 수정 * Update page.js
-(현재 애니메이션 미작동 이슈 수정중입니다)
Summary by CodeRabbit
새로운 기능
Chores
버그 수정