Conversation
Walkthrough새로운 Header UI 컴포넌트와 드롭다운 메뉴 구성요소(컨텍스트, 트리거, 메뉴, 아이템, 내비게이션 아이템) 및 배럴 파일을 추가하고, 해당 스토리북 스토리를 추가했습니다. Logo 컴포넌트와 스토리도 신규 추가했으며, Typography의 허용 태그에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 사용자
participant Header as Header
participant Trigger as MenuTrigger
participant Ctx as MenuContext
participant Menu as Menu
participant Item as MenuItem
Note over Header,Menu: 드롭다운 메뉴 제어 흐름
User->>Trigger: 클릭
Trigger->>Ctx: setOpen(!open)
Ctx-->>Menu: open=true (context 변동)
Menu->>User: 메뉴 표시(role="menu")
alt 메뉴 외부 클릭
User->>Menu: 외부 영역 클릭
Menu->>Ctx: setOpen(false)
Ctx-->>Menu: open=false
Menu-->>User: 메뉴 숨김
else 메뉴 아이템 선택
User->>Item: 클릭(role="menuitem")
Item->>Ctx: setOpen(false)
Ctx-->>Menu: open=false
Item-->>User: (href 이동 또는 onClick 실행 후 닫힘)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/ui/Typography/Typography.tsx (1)
27-30: JSDoc의 기본 weight 값과 실제 기본값이 불일치합니다.주석은
@default 'medium'인데, 실제defaultVariants.weight는'semibold'입니다. 주석 또는 기본값 중 하나를 정합성 있게 맞춰주세요.Also applies to: 46-49
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
src/shared/ui/assets/images/logo_black.pngis excluded by!**/*.pngsrc/shared/ui/assets/images/logo_white.pngis excluded by!**/*.png
📒 Files selected for processing (12)
src/shared/ui/Header/Header.stories.tsx(1 hunks)src/shared/ui/Header/Header.tsx(1 hunks)src/shared/ui/Header/Menu.tsx(1 hunks)src/shared/ui/Header/MenuContext.tsx(1 hunks)src/shared/ui/Header/MenuItem.tsx(1 hunks)src/shared/ui/Header/MenuTrigger.tsx(1 hunks)src/shared/ui/Header/NavigationItem.tsx(1 hunks)src/shared/ui/Header/index.ts(1 hunks)src/shared/ui/Logo/Logo.stories.tsx(1 hunks)src/shared/ui/Logo/Logo.tsx(1 hunks)src/shared/ui/Logo/index.ts(1 hunks)src/shared/ui/Typography/Typography.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/shared/ui/Header/Header.tsx (2)
src/shared/ui/Header/MenuItem.tsx (1)
Props(13-20)src/shared/ui/Header/NavigationItem.tsx (1)
Props(9-15)
src/shared/ui/Header/Menu.tsx (1)
src/shared/ui/Header/MenuContext.tsx (1)
useMenuCtx(16-20)
src/shared/ui/Header/MenuTrigger.tsx (1)
src/shared/ui/Header/MenuContext.tsx (1)
useMenuCtx(16-20)
src/shared/ui/Header/MenuItem.tsx (2)
src/shared/ui/Header/NavigationItem.tsx (1)
Props(9-15)src/shared/ui/Header/MenuContext.tsx (1)
useMenuCtx(16-20)
src/shared/ui/Logo/Logo.stories.tsx (1)
src/shared/ui/Logo/Logo.tsx (1)
Logo(31-43)
src/shared/ui/Header/MenuContext.tsx (3)
src/shared/ui/Header/MenuItem.tsx (1)
Props(13-20)src/shared/ui/Header/NavigationItem.tsx (1)
Props(9-15)src/shared/ui/Header/index.ts (1)
MenuContainer(3-3)
src/shared/ui/Header/NavigationItem.tsx (2)
src/shared/ui/Header/MenuItem.tsx (1)
Props(13-20)src/shared/ui/Header/index.ts (1)
NavigationItem(2-2)
src/shared/ui/Logo/Logo.tsx (1)
src/shared/ui/Logo/index.ts (1)
Logo(1-1)
🪛 GitHub Actions: Check Pull Request
src/shared/ui/Header/MenuContext.tsx
[warning] 16-16: ESLint: react-refresh/only-export-components - Fast refresh only works when a file only exports components. Use a new file to share constants or functions between components.
🔇 Additional comments (4)
src/shared/ui/Logo/index.ts (1)
1-1: 배럴 내보내기 LGTM공개 API 정리는 명확합니다.
src/shared/ui/Header/index.ts (1)
1-6: 배럴 구성 LGTM헤더 관련 퍼블릭 API가 일관되게 노출됩니다.
src/shared/ui/Header/Menu.tsx (1)
42-45: 확인 필요 —min-w-25미정의,ring-opacity-5불명확
- 검증 요약: 레포 루트의 tailwind.config.ts가 발견되었으나 extend에는 keyframes/animation만 정의되어 있으며 minWidth 또는 ringOpacity 확장 정의가 없습니다.
- 검색 상태: ripgrep 검색이 일부 스킵되어 레포 전역에서
min-w-25/ring-opacity-5사용 여부는 자동으로 확정되지 않았습니다.- 조치: src/shared/ui/Header/Menu.tsx (라인 42–45)에서 의도 확인 후, (A) tailwind.config.ts에 커스텀 minWidth(예: 25) 추가하거나, (B) 클래스 교체(예: min-w-[25px] 또는 기존 스케일)하고
ring-opacity-5가 현재 Tailwind 버전/프로젝트 규약에서 허용되는 값인지 확인하세요.src/shared/ui/Logo/Logo.stories.tsx (1)
1-49: 스토리 구성 전반 LGTM.화이트 로고 스토리의 배경 처리도 적절합니다.
Comment on lines
+39
to
+41
| <MenuItem href="notice">공지사항</MenuItem> | ||
| <MenuItem href="documents">자료실</MenuItem> | ||
| <MenuItem href="faq">FAQ</MenuItem> |
There was a problem hiding this comment.
🧹 Nitpick
Storybook에서 불필요한 네비게이션 방지(선택).
스토리 실행 중 페이지 이탈을 막으려면 데모용 href를 '#'로 바꾸거나 onClick에서 e.preventDefault() 처리하세요.
🤖 Prompt for AI Agents
src/shared/ui/Header/Header.stories.tsx around lines 39 to 41: Storybook
navigation links use real hrefs which cause page navigation during story runs;
update each MenuItem to use a harmless demo target (e.g., href="#" ) or add an
onClick handler that calls e.preventDefault() to stop navigation while
preserving the link appearance/behavior for the story.
Comment on lines
+63
to
+66
| <div className="ml-auto"> | ||
| <Icon width={20} name="list" /> | ||
| </div> | ||
| </Header> |
There was a problem hiding this comment.
Icon prop 불일치 가능성(width → size) 및 접근성 개선.
다른 사용처에서 Icon은 size를 사용합니다. 또한 인터랙션 의도를 명확히 하려면 버튼 래핑과 라벨을 권장합니다.
- <div className="ml-auto">
- <Icon width={20} name="list" />
- </div>
+ <div className="ml-auto">
+ <button type="button" aria-label="메뉴 열기">
+ <Icon size={20} name="list" aria-hidden />
+ </button>
+ </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.
Suggested change
| <div className="ml-auto"> | |
| <Icon width={20} name="list" /> | |
| </div> | |
| </Header> | |
| <div className="ml-auto"> | |
| <button type="button" aria-label="메뉴 열기"> | |
| <Icon size={20} name="list" aria-hidden /> | |
| </button> | |
| </div> | |
| </Header> |
🤖 Prompt for AI Agents
In src/shared/ui/Header/Header.stories.tsx around lines 63 to 66, the Icon usage
should match other usages (use size instead of width) and be made accessible:
replace the Icon prop width={20} with size={20}, wrap the Icon in an interactive
element (e.g., <button type="button">) preserving the existing container
className, and add an accessible label via aria-label or a visually-hidden text
node inside the button; ensure the button is keyboard-focusable and has any
necessary onClick handler or is marked appropriately if purely decorative.
Comment on lines
+5
to
+14
| export type Props = { | ||
| /** | ||
| * The content to display inside the header. | ||
| */ | ||
| children: React.ReactNode; | ||
| /** | ||
| * Additional CSS classes to apply to the header. | ||
| */ | ||
| className?: string; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick
Header에 네이티브 속성 전달(…rest) 허용을 권장합니다.
id, role, aria-*, data-*, style 등을 전달할 수 있게 타입/프롭스를 확장하세요.
+'use client';
-import { cn } from '@/shared/lib/core';
+import { ComponentPropsWithoutRef } from 'react';
+import { cn } from '@/shared/lib/core';
-export type Props = {
- /**
- * The content to display inside the header.
- */
- children: React.ReactNode;
- /**
- * Additional CSS classes to apply to the header.
- */
- className?: string;
-};
+export type Props = ComponentPropsWithoutRef<'header'> & {
+ children: React.ReactNode;
+};
-export function Header({ children, className }: Props) {
+export function Header({ children, className, ...rest }: Props) {
return (
- <header className={cn('fixed top-0 z-40 w-full border-b border-gray-200 bg-white', className)}>
+ <header {...rest} className={cn('fixed top-0 z-40 w-full border-b border-gray-200 bg-white', className)}>
<div className="mx-auto flex h-16 w-full max-w-6xl items-center px-4 md:px-8">{children}</div>
</header>
);
}🤖 Prompt for AI Agents
In src/shared/ui/Header/Header.tsx around lines 5 to 14, the Props type
currently only lists children and className and should accept native HTML
attributes (id, role, aria-*, data-*, style, etc.); update the Props definition
to extend the appropriate React HTML attributes type (e.g.,
React.HTMLAttributes<HTMLElement> or React.ComponentPropsWithoutRef<'header'>)
and include children and className, then update the Header component to accept
and spread the remaining props (…rest) onto the root header element so native
attributes are forwarded.
Comment on lines
+18
to
+32
| useEffect(() => { | ||
| function onDocClick(e: MouseEvent) { | ||
| if (!ref.current) return; | ||
| const target = e.target as Node; | ||
| if (!ref.current.contains(target)) { | ||
| const triggerElement = document.getElementById(triggerId); | ||
| if (triggerElement && triggerElement.contains(target)) { | ||
| return; | ||
| } | ||
| setOpen(false); | ||
| } | ||
| } | ||
| if (open) document.addEventListener('mousedown', onDocClick); | ||
| return () => document.removeEventListener('mousedown', onDocClick); | ||
| }, [open, setOpen, triggerId]); |
There was a problem hiding this comment.
🧹 Nitpick
Esc 키로 닫기 지원 추가 권장
키보드 사용자를 위해 열림 상태에서 Escape로 닫히도록 처리해주세요.
useEffect(() => {
function onDocClick(e: MouseEvent) {
@@
if (open) document.addEventListener('mousedown', onDocClick);
return () => document.removeEventListener('mousedown', onDocClick);
}, [open, setOpen, triggerId]);
+
+ useEffect(() => {
+ if (!open) return;
+ const onKeyDown = (e: KeyboardEvent) => {
+ if (e.key === 'Escape') setOpen(false);
+ };
+ document.addEventListener('keydown', onKeyDown);
+ return () => document.removeEventListener('keydown', onKeyDown);
+ }, [open, setOpen]);📝 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.
Suggested change
| useEffect(() => { | |
| function onDocClick(e: MouseEvent) { | |
| if (!ref.current) return; | |
| const target = e.target as Node; | |
| if (!ref.current.contains(target)) { | |
| const triggerElement = document.getElementById(triggerId); | |
| if (triggerElement && triggerElement.contains(target)) { | |
| return; | |
| } | |
| setOpen(false); | |
| } | |
| } | |
| if (open) document.addEventListener('mousedown', onDocClick); | |
| return () => document.removeEventListener('mousedown', onDocClick); | |
| }, [open, setOpen, triggerId]); | |
| useEffect(() => { | |
| function onDocClick(e: MouseEvent) { | |
| if (!ref.current) return; | |
| const target = e.target as Node; | |
| if (!ref.current.contains(target)) { | |
| const triggerElement = document.getElementById(triggerId); | |
| if (triggerElement && triggerElement.contains(target)) { | |
| return; | |
| } | |
| setOpen(false); | |
| } | |
| } | |
| if (open) document.addEventListener('mousedown', onDocClick); | |
| return () => document.removeEventListener('mousedown', onDocClick); | |
| }, [open, setOpen, triggerId]); | |
| useEffect(() => { | |
| if (!open) return; | |
| const onKeyDown = (e: KeyboardEvent) => { | |
| if (e.key === 'Escape') setOpen(false); | |
| }; | |
| document.addEventListener('keydown', onKeyDown); | |
| return () => document.removeEventListener('keydown', onKeyDown); | |
| }, [open, setOpen]); |
🤖 Prompt for AI Agents
In src/shared/ui/Header/Menu.tsx around lines 18 to 32, the useEffect only
listens for mousedown to close the menu and should also close the menu when the
user presses Escape; add a keydown listener when open that checks for e.key ===
'Escape' (or e.keyCode === 27 as fallback) and calls setOpen(false), ensure the
listener is registered only while open and removed in the cleanup along with the
existing mousedown listener, and avoid referencing stale state by using the same
setOpen from props in the handler.
| @@ -0,0 +1,38 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
🧹 Nitpick
🧩 Analysis chain
react-refresh/only-export-components 경고 처리.
이 파일이 컴포넌트 외(훅/타입)를 함께 export해서 경고가 발생했습니다. 빠른 해결로 파일 상단에 룰 비활성화를 두거나(권장도 낮음), 컴포넌트(MenuContainer)와 훅(useMenuCtx)을 파일 분리하세요(권장).
빠른 완화(주석 비활성화) 제안:
'use client';
+/* eslint-disable react-refresh/only-export-components */선호안: MenuContainer를 MenuContainer.tsx로 분리하고, 이 파일은 컨텍스트/훅만 export. 원하시면 분리 패치도 드리겠습니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="src/shared/ui/Header/MenuContext.tsx"
echo "PWD: $(pwd)"
if [ -f "$f" ]; then
echo "FOUND:$f"
echo "---- HEAD (first 240 lines) ----"
sed -n '1,240p' "$f" || true
echo "---- EXPORT LINES ----"
rg -n "export " "$f" || true
echo "---- LINES WITH 'use client' or createContext or createContext' ----"
rg -n "use client|createContext|createContext\\(" "$f" || true
else
echo "FILE_NOT_FOUND:$f"
echo "Searching repository for MenuContext/MenuContainer/useMenuCtx..."
rg -n --hidden --glob '!node_modules' "MenuContext" || true
rg -n --hidden --glob '!node_modules' "MenuContainer" || true
rg -n --hidden --glob '!node_modules' "useMenuCtx" || true
fiLength of output: 1395
react-refresh/only-export-components 경고 — 파일 분리 권장 (임시 ESLint 비활성화 옵션 포함)
src/shared/ui/Header/MenuContext.tsx가 컴포넌트(MenuContainer)와 훅(useMenuCtx)/타입을 함께 export하여 해당 ESLint 규칙이 경고를 발생시킵니다.
- 권장: MenuContainer를 src/shared/ui/Header/MenuContainer.tsx로 분리하고, 현재 파일은 컨텍스트/훅/타입만 export.
- 단기완화(권장 안 함): 파일 상단에 ESLint 규칙 비활성화 추가:
'use client';
+/* eslint-disable react-refresh/only-export-components */원하면 분리 패치 제공.
📝 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.
Suggested change
| 'use client'; | |
| 'use client'; | |
| /* eslint-disable react-refresh/only-export-components */ |
🤖 Prompt for AI Agents
In src/shared/ui/Header/MenuContext.tsx around line 1, the file currently
exports a React component (MenuContainer) together with the context/hook/types
which triggers the react-refresh/only-export-components ESLint warning; split
MenuContainer into a new file src/shared/ui/Header/MenuContainer.tsx and leave
MenuContext.tsx exporting only the context, useMenuCtx hook, and related types;
update all imports that referenced MenuContainer to point to the new path and
ensure MenuContext.tsx retains only non-component exports (remove any component
export), and do not add an ESLint-disable unless absolutely necessary.
Comment on lines
+19
to
+27
| <Caption1 | ||
| as="a" | ||
| {...(href ? { href } : { type: 'button', onClick })} | ||
| className={cn( | ||
| 'hover:text-primary-300 inline-flex items-center gap-2 rounded-md px-4 py-2 text-sm font-medium whitespace-nowrap text-neutral-400 transition-colors duration-200', | ||
| active && 'text-primary-500', | ||
| className | ||
| )} | ||
| aria-current={active ? 'page' : undefined} |
There was a problem hiding this comment.
href가 없는데도 항상 로 렌더링됨 → 접근성/시맨틱 버그
as="a" 고정 + type="button"을 <a>에 넣고 있어 HTML 시맨틱/접근성 위반입니다. href가 없으면 <button>으로 렌더링해야 합니다. 또한 onClick이 전달돼도 href가 있으면 무시되고 있어요.
아래처럼 수정하면 시맨틱/접근성/타이핑 모두 정리됩니다. (선행: Typography의 AllowedTag에 'button' 추가)
- <Caption1
- as="a"
- {...(href ? { href } : { type: 'button', onClick })}
+ <Caption1
+ as={href ? 'a' : 'button'}
+ weight="medium"
+ {...(href ? { href, onClick } : { type: 'button', onClick })}
className={cn(
- 'hover:text-primary-300 inline-flex items-center gap-2 rounded-md px-4 py-2 text-sm font-medium whitespace-nowrap text-neutral-400 transition-colors duration-200',
+ 'hover:text-primary-300 inline-flex items-center gap-2 rounded-md px-4 py-2 whitespace-nowrap text-neutral-400 transition-colors duration-200',
active && 'text-primary-500',
className
)}
- aria-current={active ? 'page' : undefined}
+ aria-current={active && href ? 'page' : undefined}
>비고:
weight="medium"으로 가중치 제어하고, 중복되는text-sm font-medium제거해 우선순위 충돌을 방지했습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/shared/ui/Header/NavigationItem.tsx around lines 19 to 27, the component
currently forces as="a" and spreads { type: 'button', onClick } even when href
is absent, causing semantic and accessibility issues; change the rendered tag to
be conditional (use 'a' only when href is present, otherwise 'button'), only
pass href when rendering an anchor and only pass type="button" when rendering a
button, ensure onClick is forwarded for the button case (and can remain for
anchors if desired), update Typography.AllowedTag to include 'button' and remove
duplicate font-weight classes (use weight="medium" instead) to fix typing and
styling conflicts.
Comment on lines
+15
to
+17
| size: { | ||
| control: { type: 'range', min: 20, max: 100, step: 5 }, | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick
argTypes 범위와 스토리 args 불일치(컨트롤 값 초과).
size 컨트롤 max가 100인데 스토리는 200을 사용합니다. 컨트롤을 200으로 올리거나 스토리 값을 100으로 낮추세요. 컨트롤 확장 권장.
- size: {
- control: { type: 'range', min: 20, max: 100, step: 5 },
- },
+ size: {
+ control: { type: 'range', min: 20, max: 200, step: 5 },
+ },Also applies to: 30-35, 37-41
🤖 Prompt for AI Agents
In src/shared/ui/Logo/Logo.stories.tsx around lines 15-17 (also review lines
30-35 and 37-41), the story uses a size arg value (200) that exceeds the
argTypes control max (100); update either the control max to 200 or reduce the
story args to be <=100 so they match — pick one approach and apply consistently
across the other story entries (adjust control.type.range.max to 200 if you want
larger sizes, or set each story's size arg to 100 or lower).
Comment on lines
+5
to
+14
| export type Props = { | ||
| /** | ||
| * The text content to display next to the logo. | ||
| */ | ||
| children?: React.ReactNode; | ||
| /** | ||
| * The URL to navigate to when the logo is clicked. | ||
| * @default '/' | ||
| */ | ||
| href?: string; |
There was a problem hiding this comment.
🧹 Nitpick
네이티브 속성 전달(…rest) 허용을 권장합니다.
소비처가 target, rel, aria-*, data-* 등을 넘길 수 있도록 앵커 속성 확장을 추천합니다.
-import { cn } from '@/shared/lib/core';
+import { ComponentPropsWithoutRef } from 'react';
+import { cn } from '@/shared/lib/core';
@@
-export type Props = {
+export type Props = ComponentPropsWithoutRef<'a'> & {
/**
* The text content to display next to the logo.
*/
children?: React.ReactNode;
/**
* The URL to navigate to when the logo is clicked.
* @default '/'
*/
href?: string;
@@
-};
+};
@@
-export function Logo({ children, href = '/', className, color = 'black', size = 40 }: Props) {
+export function Logo({ children, href = '/', className, color = 'black', size = 40, ...rest }: Props) {
@@
- <a href={href} className={cn('inline-flex items-center', className)} aria-label="홈으로 이동">
+ <a href={href} {...rest} className={cn('inline-flex items-center', className)} aria-label="홈으로 이동">📝 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.
Suggested change
| export type Props = { | |
| /** | |
| * The text content to display next to the logo. | |
| */ | |
| children?: React.ReactNode; | |
| /** | |
| * The URL to navigate to when the logo is clicked. | |
| * @default '/' | |
| */ | |
| href?: string; | |
| import { ComponentPropsWithoutRef } from 'react'; | |
| import { cn } from '@/shared/lib/core'; | |
| export type Props = ComponentPropsWithoutRef<'a'> & { | |
| /** | |
| * The text content to display next to the logo. | |
| */ | |
| children?: React.ReactNode; | |
| /** | |
| * The URL to navigate to when the logo is clicked. | |
| * @default '/' | |
| */ | |
| href?: string; | |
| }; | |
| export function Logo({ children, href = '/', className, color = 'black', size = 40, ...rest }: Props) { | |
| return ( | |
| <a href={href} {...rest} className={cn('inline-flex items-center', className)} aria-label="홈으로 이동"> | |
| {/* logo markup */} | |
| {children} | |
| </a> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/shared/ui/Logo/Logo.tsx around lines 5 to 14, the Props type only
declares children and href so consumers cannot pass native anchor attributes
like target, rel, aria-*, data-*, etc.; change Props to allow anchor attributes
(e.g., extend React.AnchorHTMLAttributes<HTMLAnchorElement> or add an index
signature) and update the Logo component signature to collect and forward a
...rest spread into the <a> element so all native attributes passed by consumers
are applied to the anchor.
Comment on lines
+31
to
+41
| export function Logo({ children, href = '/', className, color = 'black', size = 40 }: Props) { | ||
| const logoSrc = | ||
| color === 'white' | ||
| ? '/src/shared/ui/assets/images/logo_white.png' | ||
| : '/src/shared/ui/assets/images/logo_black.png'; | ||
|
|
||
| return ( | ||
| <a href={href} className={cn('inline-flex items-center', className)} aria-label="홈으로 이동"> | ||
| <img src={logoSrc} alt="ddingdong Logo" width={size} height={size} /> | ||
| {children && <span className="ml-2 text-lg font-semibold">{children}</span>} | ||
| </a> |
There was a problem hiding this comment.
절대 경로 이미지(/src/**)는 빌드에서 깨질 수 있음 — 번들러 임포트로 교체하세요.
라이브러리/앱 환경별로 /src/...png는 동작이 달라집니다. 정적 자산은 모듈 임포트(또는 public 경로)로 해결하는 게 안전합니다. 또한 이미지에 합리적 기본 속성 추가를 권장합니다.
'use client';
import { cn } from '@/shared/lib/core';
+import logoWhite from '@/shared/ui/assets/images/logo_white.png';
+import logoBlack from '@/shared/ui/assets/images/logo_black.png';
@@
-export function Logo({ children, href = '/', className, color = 'black', size = 40 }: Props) {
- const logoSrc =
- color === 'white'
- ? '/src/shared/ui/assets/images/logo_white.png'
- : '/src/shared/ui/assets/images/logo_black.png';
+export function Logo({ children, href = '/', className, color = 'black', size = 40 }: Props) {
+ const logoSrc = color === 'white' ? logoWhite : logoBlack;
@@
- <img src={logoSrc} alt="ddingdong Logo" width={size} height={size} />
+ <img
+ src={logoSrc}
+ alt="ddingdong Logo"
+ width={size}
+ height={size}
+ loading="lazy"
+ decoding="async"
+ draggable={false}
+ />📝 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.
Suggested change
| export function Logo({ children, href = '/', className, color = 'black', size = 40 }: Props) { | |
| const logoSrc = | |
| color === 'white' | |
| ? '/src/shared/ui/assets/images/logo_white.png' | |
| : '/src/shared/ui/assets/images/logo_black.png'; | |
| return ( | |
| <a href={href} className={cn('inline-flex items-center', className)} aria-label="홈으로 이동"> | |
| <img src={logoSrc} alt="ddingdong Logo" width={size} height={size} /> | |
| {children && <span className="ml-2 text-lg font-semibold">{children}</span>} | |
| </a> | |
| 'use client'; | |
| import { cn } from '@/shared/lib/core'; | |
| import logoWhite from '@/shared/ui/assets/images/logo_white.png'; | |
| import logoBlack from '@/shared/ui/assets/images/logo_black.png'; | |
| export function Logo({ children, href = '/', className, color = 'black', size = 40 }: Props) { | |
| const logoSrc = color === 'white' ? logoWhite : logoBlack; | |
| return ( | |
| <a href={href} className={cn('inline-flex items-center', className)} aria-label="홈으로 이동"> | |
| <img | |
| src={logoSrc} | |
| alt="ddingdong Logo" | |
| width={size} | |
| height={size} | |
| loading="lazy" | |
| decoding="async" | |
| draggable={false} | |
| /> | |
| {children && <span className="ml-2 text-lg font-semibold">{children}</span>} | |
| </a> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/shared/ui/Logo/Logo.tsx around lines 31-41, the component uses absolute
file paths like "/src/...png" which will break at build time; replace them with
module imports or a public static path and add sensible image attributes. Import
both logo assets at the top (e.g. import logoWhite from '.../logo_white.png' and
logoBlack from '.../logo_black.png') and switch logoSrc to reference those
imports (or, if you prefer public/, move images to public/ and use
"/logo_white.png" and "/logo_black.png"). Also add reasonable defaults on the
<img> such as loading="lazy" and decoding="async" (keep existing alt and
explicit width/height), and ensure the import paths are correct for your bundler
so the images are emitted in the build.
| }); | ||
|
|
||
| type AllowedTag = 'h1' | 'h2' | 'h3' | 'p' | 'div' | 'label' | 'span'; | ||
| type AllowedTag = 'h1' | 'h2' | 'h3' | 'p' | 'div' | 'label' | 'span' | 'a'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
버튼 태그도 허용하세요 — NavigationItem을 시맨틱하게 렌더링하려면 필수입니다.
NavigationItem이 href 없을 때도 <a>로 렌더링되는 문제가 있어요. Typography가 <button>을 허용하면 href 없을 때 버튼으로 렌더링해 접근성과 시맨틱이 개선됩니다.
-type AllowedTag = 'h1' | 'h2' | 'h3' | 'p' | 'div' | 'label' | 'span' | 'a';
+type AllowedTag = 'h1' | 'h2' | 'h3' | 'p' | 'div' | 'label' | 'span' | 'a' | 'button';📝 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.
Suggested change
| type AllowedTag = 'h1' | 'h2' | 'h3' | 'p' | 'div' | 'label' | 'span' | 'a'; | |
| type AllowedTag = 'h1' | 'h2' | 'h3' | 'p' | 'div' | 'label' | 'span' | 'a' | 'button'; |
🤖 Prompt for AI Agents
In src/shared/ui/Typography/Typography.tsx around line 33, the AllowedTag union
lacks 'button', which prevents rendering semantic button elements for
NavigationItem; add 'button' to the AllowedTag union and update the component's
prop typing so when tag === 'button' the props accept ButtonHTMLAttributes (or
adjust the generic element props mapping) to allow onClick/type/etc., ensuring
correct runtime rendering and accessibility.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔥 연관 이슈
🚀 작업 내용
🤔 고민했던 내용
herf속성을 가지고 있는데 이가 적절해보이는지 궁금합니다!💬 리뷰 중점사항
Summary by CodeRabbit
신기능
문서
리팩터