Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Walkthrough이 PR은 멘토 수정 페이지의 채널 입력 영역을 개선하고 관련 UI 요소들을 다듬는 변경사항을 포함합니다.
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aab9a147d4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const completedCount = (watchedChannels ?? []).filter( | ||
| (channel) => channel?.type !== null && channel?.type !== undefined && Boolean(channel?.url?.trim()), | ||
| ).length; | ||
| const visibleCount = Math.min(completedCount + 1, MAX_CHANNELS); |
There was a problem hiding this comment.
When a later channel is selected but not completed, reducing an earlier channel can make completedCount drop and unmount that later field even though React Hook Form keeps its value by default. For example, complete channel 1, select a type for channel 2 without a URL, then clear channel 1: channel 2 disappears, but the schema still rejects the hidden type without a URL on submit, leaving the user blocked with no visible error to fix.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/components/mentor/ChannelSelct/index.tsx (1)
40-40: ⚡ Quick win드롭다운 메뉴의 테두리 색상도 통일하는 것을 고려해보세요.
현재 토글 버튼(Line 30)의 테두리는
border-k-100으로 변경되었지만, 드롭다운 메뉴 컨테이너는 여전히border-gray-300을 사용하고 있습니다. 레이어 목표("테두리 색상을 border-k-100으로 통일")를 완전히 달성하려면 이 부분도border-k-100으로 변경하는 것이 일관성 측면에서 좋을 것 같습니다.♻️ 스타일 통일을 위한 제안
- <div className="absolute left-0 right-0 top-full z-50 mt-1 rounded-lg border border-gray-300 bg-white shadow-lg"> + <div className="absolute left-0 right-0 top-full z-50 mt-1 rounded-lg border border-k-100 bg-white shadow-lg">🤖 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/components/mentor/ChannelSelct/index.tsx` at line 40, The dropdown container in ChannelSelct (the div with class "absolute left-0 right-0 top-full z-50 mt-1 rounded-lg border border-gray-300 bg-white shadow-lg") uses border-gray-300 while the toggle button was changed to border-k-100; update that container's border class to border-k-100 so the dropdown and toggle share the same border styling for consistency.apps/web/src/styles/globals.css (1)
87-100: ⚡ Quick win1) 모션 접근성을 위해
prefers-reduced-motion분기를 같이 두는 게 좋습니다.새 애니메이션 도입 시, 사용자 OS 설정에 따라 애니메이션을 끌 수 있게 하면 접근성이 더 좋아집니다.
개선 예시
.animate-channel-reveal { animation: channel-reveal 0.8s ease-out; } + +@media (prefers-reduced-motion: reduce) { + .animate-channel-reveal { + animation: none; + } +}🤖 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/styles/globals.css` around lines 87 - 100, Add a prefers-reduced-motion branch so users who disable motion in their OS won't see the animation: wrap a media query for (prefers-reduced-motion: reduce) and inside override the .animate-channel-reveal animation (and/or keyframes) to none or set animation-duration: 0s / animation: none; keep the existing `@keyframes` channel-reveal and .animate-channel-reveal unchanged for default behavior but ensure the media query targets .animate-channel-reveal to disable the animation when reduced motion is requested.
🤖 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/mentor/modify/_ui/ModifyContent/_ui/ChannelBox/index.tsx`:
- Around line 18-26: The current visibleCount calculation uses completedCount
which hides non-contiguous saved channel slots; change it to compute the highest
filled channel index (scan watchedChannels or channels for first/last index
where channel?.type or channel?.url is present), set visibleCount =
Math.min(maxFilledIndex + 2, MAX_CHANNELS) so we always render up to the next
slot after the last populated index (and default to 1 when none filled). Update
the logic around completedCount/visibleCount (references: completedCount,
visibleCount, MAX_CHANNELS, ChannelItem, channels) to use the max-index approach
and keep ChannelItem rendering loop unchanged.
---
Nitpick comments:
In `@apps/web/src/components/mentor/ChannelSelct/index.tsx`:
- Line 40: The dropdown container in ChannelSelct (the div with class "absolute
left-0 right-0 top-full z-50 mt-1 rounded-lg border border-gray-300 bg-white
shadow-lg") uses border-gray-300 while the toggle button was changed to
border-k-100; update that container's border class to border-k-100 so the
dropdown and toggle share the same border styling for consistency.
In `@apps/web/src/styles/globals.css`:
- Around line 87-100: Add a prefers-reduced-motion branch so users who disable
motion in their OS won't see the animation: wrap a media query for
(prefers-reduced-motion: reduce) and inside override the .animate-channel-reveal
animation (and/or keyframes) to none or set animation-duration: 0s / animation:
none; keep the existing `@keyframes` channel-reveal and .animate-channel-reveal
unchanged for default behavior but ensure the media query targets
.animate-channel-reveal to disable the animation when reduced motion is
requested.
🪄 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: f423903b-cd7a-449e-b80e-ff4f9661102f
📒 Files selected for processing (8)
apps/web/src/app/mentor/layout.tsxapps/web/src/app/mentor/modify/_ui/ModifyContent/_ui/AddArticleCard/index.tsxapps/web/src/app/mentor/modify/_ui/ModifyContent/_ui/ArticlePanel/index.tsxapps/web/src/app/mentor/modify/_ui/ModifyContent/_ui/ChannelBox/ChannelItem.tsxapps/web/src/app/mentor/modify/_ui/ModifyContent/_ui/ChannelBox/index.tsxapps/web/src/app/mentor/modify/_ui/ModifyContent/index.tsxapps/web/src/components/mentor/ChannelSelct/index.tsxapps/web/src/styles/globals.css
| const completedCount = (watchedChannels ?? []).filter( | ||
| (channel) => channel?.type !== null && channel?.type !== undefined && Boolean(channel?.url?.trim()), | ||
| ).length; | ||
| const visibleCount = Math.min(completedCount + 1, MAX_CHANNELS); | ||
|
|
||
| return ( | ||
| <> | ||
| {Array.from({ length: 4 }, (_, index) => ( | ||
| {Array.from({ length: visibleCount }, (_, index) => ( | ||
| <ChannelItem key={index} index={index} channel={channels[index]} /> |
There was a problem hiding this comment.
1) completedCount 기준 노출 계산은 비연속 저장 채널을 숨길 수 있습니다.
현재 방식은 “완료된 개수”만 보므로, 기존 데이터가 비연속일 때(예: 3번만 값 존재) 해당 슬롯이 렌더링되지 않아 수정 경로가 사라집니다. 완료 인덱스의 최댓값 기준으로 다음 슬롯까지 보여주도록 바꾸는 편이 안전합니다.
수정 예시
- const completedCount = (watchedChannels ?? []).filter(
- (channel) => channel?.type !== null && channel?.type !== undefined && Boolean(channel?.url?.trim()),
- ).length;
- const visibleCount = Math.min(completedCount + 1, MAX_CHANNELS);
+ const lastCompletedIndex = (watchedChannels ?? []).reduce((last, channel, index) => {
+ const isCompleted = channel?.type != null && Boolean(channel?.url?.trim());
+ return isCompleted ? index : last;
+ }, -1);
+ const visibleCount = Math.min(Math.max(lastCompletedIndex + 2, 1), MAX_CHANNELS);🤖 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/mentor/modify/_ui/ModifyContent/_ui/ChannelBox/index.tsx`
around lines 18 - 26, The current visibleCount calculation uses completedCount
which hides non-contiguous saved channel slots; change it to compute the highest
filled channel index (scan watchedChannels or channels for first/last index
where channel?.type or channel?.url is present), set visibleCount =
Math.min(maxFilledIndex + 2, MAX_CHANNELS) so we always render up to the next
slot after the last populated index (and default to 1 when none filled). Update
the logic around completedCount/visibleCount (references: completedCount,
visibleCount, MAX_CHANNELS, ChannelItem, channels) to use the max-index approach
and keep ChannelItem rendering loop unchanged.
manNomi
left a comment
manNomi
left a comment
There was a problem hiding this comment.
전반적인 디자인 수정이네요 너무 고생하셨습니다
버셀 오류 나오는건 멀티존 반영사항이 해당 PR에 반영안되어서 그런것이라 문제없어보입니다
|
브랜치명을 조금더 다르게 가져가도 좋을것 같아요 |
2 participants
작업 내용
멘토 정보 수정 페이지 UI 수정
추가 수정 내용
내 채널 n태그 채널별 색상 적용새로운 아티클 추가하기영역 가운데 정렬멘토 수정>멘토 페이지로 제목 수정리뷰 요구사항
수경님이 요구해주신 내용 이외에 피그마 디자인과 다른 부분을 찾아서 추가로 수정하였습니다!
추가적인 수정 부분이나 피드백 주시면 반영해보겠습니다!