Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📋 전체 변경 사항이 PR은 공유 기능 UI를 개선하고 아티클 미리보기 카드를 도입하는 변경사항을 담고 있습니다. 공유 아이콘 자산을 등록한 후 대학 상세 페이지의 공유 버튼을 새 아이콘으로 업데이트하고, 아이콘이 클릭 시 600ms간 상태 전환되는 시각적 피드백을 추가합니다. 동시에 아티클 목록 조회 훅에 조건부 활성화 옵션을 확장하고, 아티클 카드를 렌더링하는 새로운 컴포넌트를 추가합니다. 🚶 Walkthrough
🎯 코드 리뷰 예상 난이도🎯 2 (Simple) | ⏱️ ~12 분 💭 제안 리뷰어
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 2
🧹 Nitpick comments (1)
apps/web/src/apis/news/getNewsList.ts (1)
20-20: 💤 Low value타입 시그니처와 일치하도록 중복 체크를 제거하는 것이 좋겠습니다.
userId의 타입이number로 선언되어 있어null이 될 수 없으므로,userId !== null체크는 중복입니다. 다음과 같이 간소화할 수 있습니다:♻️ 제안하는 수정안
- enabled: userId !== null && userId !== 0 && (options?.enabled ?? true), + enabled: userId !== 0 && (options?.enabled ?? true),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/apis/news/getNewsList.ts` at line 20, The enabled condition includes a redundant null check for userId; since userId is typed as number (non-nullable), remove the userId !== null clause and simplify the expression in the enabled assignment (the property named enabled in apps/web/src/apis/news/getNewsList.ts) to only check userId !== 0 && (options?.enabled ?? true).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/web/src/app/university/`[homeUniversity]/[id]/_ui/UniversityDetail/_ui/UniversityBtns.tsx:
- Around line 80-95: The share button lacks a visible keyboard focus indicator;
update the button element that uses onClick={handleCopy} and the
isShareActive/IconShare/IconShareFilled visuals to include the same focus ring
classes used by the like button (e.g., add focus:ring-2 focus:ring-white/50 and
remove or replace focus:outline-none) so keyboard users see a clear focus state
while preserving the current hover/active animations and icon opacity toggles.
- Around line 54-59: handleCopy currently calls
navigator.clipboard.writeText(...).then(() => {}) without error handling and
creates a potential memory leak by using setTimeout without cleanup; update
handleCopy to await or use .then/.catch on navigator.clipboard.writeText to show
a success toast on resolve and an error toast on reject (include clear
messaging), and replace the raw setTimeout with a tracked timeoutId (e.g., store
id in a ref or state) so you can call clearTimeout during unmount; add a
useEffect cleanup that clears the timeoutId and prevents calling
setIsShareActive on an unmounted component.
---
Nitpick comments:
In `@apps/web/src/apis/news/getNewsList.ts`:
- Line 20: The enabled condition includes a redundant null check for userId;
since userId is typed as number (non-nullable), remove the userId !== null
clause and simplify the expression in the enabled assignment (the property named
enabled in apps/web/src/apis/news/getNewsList.ts) to only check userId !== 0 &&
(options?.enabled ?? true).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9d0e914f-3443-4977-9d6b-efcf4c34a6ec
⛔ Files ignored due to path filters (2)
apps/web/public/svgs/shareIcon.svgis excluded by!**/*.svgapps/web/public/svgs/shareIconFilled.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
apps/web/public/svgs/index.tsapps/web/src/apis/news/getNewsList.tsapps/web/src/app/university/[homeUniversity]/[id]/_ui/UniversityDetail/_ui/UniversityBtns.tsxapps/web/src/components/mentor/MentorCard/_ui/ArticlePreview.tsx
Comment on lines
54
to
59
| const handleCopy = () => { | ||
| navigator.clipboard.writeText(window.location.href).then(() => {}); | ||
| toast.success("URL이 복사되었습니다."); | ||
| setIsShareActive(true); | ||
| setTimeout(() => setIsShareActive(false), 600); | ||
| }; |
There was a problem hiding this comment.
공유 기능 구현에서 두 가지 개선이 필요합니다.
다음 사항들을 확인해 주세요:
-
클립보드 API 에러 처리 누락 (Line 55)
navigator.clipboard.writeText()의 빈.then()콜백이 불필요하며, 실패 시 처리를 위한.catch()핸들러가 없습니다.- 클립보드 API는 권한 문제나 브라우저 제약으로 실패할 수 있습니다.
-
메모리 누수 가능성 (Lines 57-58)
setTimeout이 정리(cleanup)되지 않아, 600ms 이내에 컴포넌트가 언마운트되면 언마운트된 컴포넌트에 상태 업데이트를 시도하게 됩니다.- React에서 흔히 발생하는 안티패턴입니다.
🔧 개선된 구현 제안
- const handleCopy = () => {
- navigator.clipboard.writeText(window.location.href).then(() => {});
- toast.success("URL이 복사되었습니다.");
- setIsShareActive(true);
- setTimeout(() => setIsShareActive(false), 600);
- };
+ const handleCopy = () => {
+ navigator.clipboard
+ .writeText(window.location.href)
+ .then(() => {
+ toast.success("URL이 복사되었습니다.");
+ setIsShareActive(true);
+ })
+ .catch(() => {
+ toast.error("URL 복사에 실패했습니다.");
+ });
+ };
+
+ useEffect(() => {
+ if (!isShareActive) return;
+ const timer = setTimeout(() => setIsShareActive(false), 600);
+ return () => clearTimeout(timer);
+ }, [isShareActive]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/web/src/app/university/`[homeUniversity]/[id]/_ui/UniversityDetail/_ui/UniversityBtns.tsx
around lines 54 - 59, handleCopy currently calls
navigator.clipboard.writeText(...).then(() => {}) without error handling and
creates a potential memory leak by using setTimeout without cleanup; update
handleCopy to await or use .then/.catch on navigator.clipboard.writeText to show
a success toast on resolve and an error toast on reject (include clear
messaging), and replace the raw setTimeout with a tracked timeoutId (e.g., store
id in a ref or state) so you can call clearTimeout during unmount; add a
useEffect cleanup that clears the timeoutId and prevents calling
setIsShareActive on an unmounted component.
Comment on lines
80
to
95
| <button | ||
| type="button" | ||
| onClick={handleCopy} | ||
| className={`/* stroke: #FFF; stroke-width: 1px; */ /* fill: linear-gradient(...) */ /* CSS의 fill은 SVG 속성이지만, 버튼 배경으로 적용합니다. */ /* backdrop-filter: blur(2px); */ /* filter: drop-shadow(...) */ /* 기타 스타일 */ rounded-full border border-white/80 bg-[linear-gradient(136deg,rgba(255,255,255,0.4)_14.87%,rgba(199,212,250,0.8)_89.1%)] p-3 drop-shadow-[2px_2px_6px_#C7D4FA] backdrop-blur-[2px] transition-transform duration-200 ease-in-out hover:scale-110 focus:outline-none focus:ring-2 focus:ring-white/50 active:scale-95`} | ||
| className="relative h-10 w-10 transition-transform duration-200 ease-in-out hover:scale-110 focus:outline-none active:scale-95" | ||
| > | ||
| {copyIcon} | ||
| <IconShare | ||
| width={40} | ||
| height={40} | ||
| className={`absolute inset-0 transition-opacity duration-300 ease-in-out ${isShareActive ? "opacity-0" : "opacity-100"}`} | ||
| /> | ||
| <IconShareFilled | ||
| width={40} | ||
| height={40} | ||
| className={`absolute inset-0 transition-opacity duration-300 ease-in-out ${isShareActive ? "opacity-100" : "opacity-0"}`} | ||
| /> | ||
| </button> |
There was a problem hiding this comment.
공유 버튼의 키보드 접근성 개선이 필요합니다.
아이콘 전환 애니메이션 구현은 깔끔하지만, 버튼에 포커스 인디케이터가 없어 키보드 네비게이션 사용자가 현재 포커스 위치를 파악하기 어렵습니다.
위의 좋아요 버튼(Line 76)에는 focus:ring-2 focus:ring-white/50이 있지만, 공유 버튼에는 focus:outline-none만 있고 대체 포커스 스타일이 없습니다.
♿ 접근성 개선 제안
<button
type="button"
onClick={handleCopy}
- className="relative h-10 w-10 transition-transform duration-200 ease-in-out hover:scale-110 focus:outline-none active:scale-95"
+ className="relative h-10 w-10 rounded-full transition-transform duration-200 ease-in-out hover:scale-110 focus:outline-none focus:ring-2 focus:ring-white/50 active:scale-95"
>📝 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
| <button | |
| type="button" | |
| onClick={handleCopy} | |
| className={`/* stroke: #FFF; stroke-width: 1px; */ /* fill: linear-gradient(...) */ /* CSS의 fill은 SVG 속성이지만, 버튼 배경으로 적용합니다. */ /* backdrop-filter: blur(2px); */ /* filter: drop-shadow(...) */ /* 기타 스타일 */ rounded-full border border-white/80 bg-[linear-gradient(136deg,rgba(255,255,255,0.4)_14.87%,rgba(199,212,250,0.8)_89.1%)] p-3 drop-shadow-[2px_2px_6px_#C7D4FA] backdrop-blur-[2px] transition-transform duration-200 ease-in-out hover:scale-110 focus:outline-none focus:ring-2 focus:ring-white/50 active:scale-95`} | |
| className="relative h-10 w-10 transition-transform duration-200 ease-in-out hover:scale-110 focus:outline-none active:scale-95" | |
| > | |
| {copyIcon} | |
| <IconShare | |
| width={40} | |
| height={40} | |
| className={`absolute inset-0 transition-opacity duration-300 ease-in-out ${isShareActive ? "opacity-0" : "opacity-100"}`} | |
| /> | |
| <IconShareFilled | |
| width={40} | |
| height={40} | |
| className={`absolute inset-0 transition-opacity duration-300 ease-in-out ${isShareActive ? "opacity-100" : "opacity-0"}`} | |
| /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={handleCopy} | |
| className="relative h-10 w-10 rounded-full transition-transform duration-200 ease-in-out hover:scale-110 focus:outline-none focus:ring-2 focus:ring-white/50 active:scale-95" | |
| > | |
| <IconShare | |
| width={40} | |
| height={40} | |
| className={`absolute inset-0 transition-opacity duration-300 ease-in-out ${isShareActive ? "opacity-0" : "opacity-100"}`} | |
| /> | |
| <IconShareFilled | |
| width={40} | |
| height={40} | |
| className={`absolute inset-0 transition-opacity duration-300 ease-in-out ${isShareActive ? "opacity-100" : "opacity-0"}`} | |
| /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/web/src/app/university/`[homeUniversity]/[id]/_ui/UniversityDetail/_ui/UniversityBtns.tsx
around lines 80 - 95, The share button lacks a visible keyboard focus indicator;
update the button element that uses onClick={handleCopy} and the
isShareActive/IconShare/IconShareFilled visuals to include the same focus ring
classes used by the like button (e.g., add focus:ring-2 focus:ring-white/50 and
remove or replace focus:outline-none) so keyboard users see a clear focus state
while preserving the current hover/active animations and icon opacity toggles.
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.
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)