[Feat] #17 - Navigation 컴포넌트 구현#51
Merged
Merged
Conversation
5 tasks
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Navigation component with types, Storybook stories, and UI-barrel exports; introduces a LogOutIcon (duplicated in icons file); removes the Header component, its stories, and header exports; and updates .gitignore to add Changes
Sequence DiagramsequenceDiagram
participant Browser as User/Browser
participant Nav as Navigation Component
participant Profile as Profile Dropdown
participant Auth as Auth Handler
Browser->>Nav: Render with props (items, user?, isAdminMode?)
alt no user
Nav->>Browser: Render Login / Signup buttons
else user present
Nav->>Browser: Render Profile button (+ Admin toggle if user.isAdmin)
end
Browser->>Nav: Click Profile button
Nav->>Profile: toggle showProfile
Profile->>Browser: show dropdown with name, email, type, actions
alt user clicks Admin toggle
Nav->>Auth: onAdminToggle()
Auth-->>Nav: isAdminMode updated
Nav->>Browser: update toggle label / UI
end
Browser->>Profile: Click Logout
Profile->>Auth: onLogout()
Auth-->>Browser: perform logout / redirect
Nav->>Nav: update/render guest UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
kimsman06
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/shared/ui/navigation/navigation.tsx (2)
217-222: "내 포트폴리오" button lacks a callback prop.This button only closes the dropdown without triggering any action. Consider adding an
onPortfolioClickprop similar toonProfileClickfor consistency and to allow parent components to handle navigation.💡 Proposed fix
Add to
NavigationProps:onPortfolioClick?: () => void;Then update the button:
<button - onClick={() => setShowProfile(false)} + onClick={() => { + onPortfolioClick?.(); + setShowProfile(false); + }} className="text-left px-3 py-2.5 text-[14px] text-[var(--color-gray-900,`#1a1a1a`)] hover:bg-[var(--color-gray-100)] rounded-md transition-colors" > 내 포트폴리오 </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/navigation/navigation.tsx` around lines 217 - 222, Add an optional onPortfolioClick prop to NavigationProps and wire it into the portfolio button so the parent can react when "내 포트폴리오" is clicked: update the NavigationProps interface to include onPortfolioClick?: () => void; then modify the button's onClick handler (currently using setShowProfile(false)) to call onPortfolioClick if provided and still close the dropdown (call setShowProfile(false) after or before invoking the prop); mirror how onProfileClick is used to ensure consistent behavior and typing.
152-158: Consider addingrole="switch"andaria-checkedfor better accessibility.The toggle button has
aria-labelbut lacks semantic switch attributes that screen readers use to announce toggle state.♿ Proposed accessibility improvement
<button onClick={onAdminToggle} className={getToggleClasses(isAdminMode)} + role="switch" + aria-checked={isAdminMode} aria-label="관리자 모드 토글" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/navigation/navigation.tsx` around lines 152 - 158, The toggle button currently uses aria-label but is missing semantic switch attributes; update the button element used with onAdminToggle (the one using getToggleClasses/getToggleHandleClasses and isAdminMode) to include role="switch" and aria-checked set from isAdminMode (e.g., aria-checked={isAdminMode}) so screen readers announce its state correctly while keeping the existing onClick, className, and aria-label intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/shared/ui/navigation/navigation.tsx`:
- Around line 217-222: Add an optional onPortfolioClick prop to NavigationProps
and wire it into the portfolio button so the parent can react when "내 포트폴리오" is
clicked: update the NavigationProps interface to include onPortfolioClick?: ()
=> void; then modify the button's onClick handler (currently using
setShowProfile(false)) to call onPortfolioClick if provided and still close the
dropdown (call setShowProfile(false) after or before invoking the prop); mirror
how onProfileClick is used to ensure consistent behavior and typing.
- Around line 152-158: The toggle button currently uses aria-label but is
missing semantic switch attributes; update the button element used with
onAdminToggle (the one using getToggleClasses/getToggleHandleClasses and
isAdminMode) to include role="switch" and aria-checked set from isAdminMode
(e.g., aria-checked={isAdminMode}) so screen readers announce its state
correctly while keeping the existing onClick, className, and aria-label intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 568f039d-e6fa-47ae-89ce-e86e2ea809b6
⛔ Files ignored due to path filters (1)
public/assets/ajou-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
.gitignoresrc/shared/ui/icons/index.tsxsrc/shared/ui/index.tssrc/shared/ui/navigation/navigation.stories.tsxsrc/shared/ui/navigation/navigation.tsx
This was referenced Mar 28, 2026
5 tasks
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
2 participants
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.
[Feat] #17 - Navigation 컴포넌트 구현
🔎 What is this PR?
피그마 디자인 기반 네비게이션 컴포넌트를 shared/ui 레이어에 구현했습니다.
📝 Changes
📸 Screenshots
📚 Background / Context
✔ Checklist
🙏 Request
봐주시면 감사하겠습니다.
Summary by CodeRabbit
New Features
Tests
Removals
Chores