Feat/126 알림 표시 기능 구현 #129
Feat/126 알림 표시 기능 구현 #129
Conversation
Walkthrough
Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
evaain706
left a comment
evaain706
left a comment
There was a problem hiding this 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/Header.tsx(4 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/components/Header.tsx
[error] 85-89: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (4)
src/components/Header.tsx (4)
13-13: 알림 기능을 위한 훅 import가 적절합니다.새로운 알림 표시 기능을 위해
useNotifications훅을 import한 것이 올바릅니다.
24-25: 알림 데이터 fetching과 상태 관리가 적절합니다.
useNotifications훅을 사용하여 최신 10개의 알림을 가져오고, nullish coalescing을 통해totalCount가 undefined인 경우를 안전하게 처리하고 있습니다.
58-59: 스켈레톤 플레이스홀더의 클래스 순서 변경이 확인됩니다.CSS 클래스의 순서가 변경되었지만 시각적 효과에는 영향이 없습니다.
24-92: 전체적인 알림 표시 기능 구현이 우수합니다.
- 커스텀 훅을 통한 깔끔한 데이터 fetching
- 조건부 렌더링을 통한 효율적인 UI 표시
- 기존 컴포넌트 구조와의 자연스러운 통합
이 구현은 React 모범 사례를 잘 따르고 있으며 유지보수가 용이합니다.
| aria-label='알림' | ||
| onClick={toggleOpen} | ||
| className='hover:text-primary' | ||
| className='hover:text-primary relative' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
버튼에 명시적인 type 속성을 추가하세요.
relative 포지셔닝 추가는 알림 배지를 위해 적절하지만, 버튼에 명시적인 type="button" 속성을 추가해야 합니다.
<button
aria-label='알림'
onClick={toggleOpen}
- className='hover:text-primary relative'
+ type="button"
+ className='hover:text-primary relative'
>🧰 Tools
🪛 Biome (2.1.2)
[error] 85-89: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In src/components/Header.tsx at line 85, the button element lacks an explicit
type attribute, which can cause unintended form submissions. Add the attribute
type="button" to the button element to clearly specify its behavior as a button
and prevent default form submission.
| {hasNotification && ( | ||
| <span className='font-regular absolute top-[-11px] right-[-3px] text-[10px] text-red-500'> | ||
| {data?.totalCount} | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
알림 배지 구현이 기능적으로 올바르지만 개선사항이 있습니다.
조건부 렌더링과 위치 지정이 적절하게 구현되었습니다. 다음 개선사항을 고려해보세요:
- 큰 숫자에 대한 최대 표시 제한 (예: 99+)
- 접근성을 위한 최소 폰트 크기 준수
큰 알림 개수에 대한 처리를 위해 다음과 같이 개선할 수 있습니다:
{hasNotification && (
- <span className='font-regular absolute top-[-11px] right-[-3px] text-[10px] text-red-500'>
- {data?.totalCount}
+ <span className='font-regular absolute top-[-11px] right-[-3px] text-[11px] text-red-500'>
+ {(data?.totalCount ?? 0) > 99 ? '99+' : data?.totalCount}
</span>
)}📝 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.
| {hasNotification && ( | |
| <span className='font-regular absolute top-[-11px] right-[-3px] text-[10px] text-red-500'> | |
| {data?.totalCount} | |
| </span> | |
| )} | |
| {hasNotification && ( | |
| <span className='font-regular absolute top-[-11px] right-[-3px] text-[11px] text-red-500'> | |
| {(data?.totalCount ?? 0) > 99 ? '99+' : data?.totalCount} | |
| </span> | |
| )} |
🤖 Prompt for AI Agents
In src/components/Header.tsx around lines 88 to 92, the notification badge
currently displays the totalCount directly, which can cause layout issues with
large numbers. Modify the rendering logic to cap the displayed number at a
maximum value like "99+" when totalCount exceeds 99. Also, increase the font
size to meet accessibility standards by setting a minimum font size of at least
12px or as per design guidelines.
📌 변경 사항 개요
알림 표시 기능 구현
📝 상세 내용
총 알림 갯수를 가져와서 알림이 있을 때는 숫자로 몇개 있는지 표현하고 없으면 아무것도 안 나오도록 했습니다.
🔗 관련 이슈
🖼️ 스크린샷(선택사항)
💡 참고 사항
Summary by CodeRabbit
New Features
Style