Repository navigation
feat: 사용자 추천 프로필 클릭 시 상대방 프로필로 이동 구현 - #261
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements profile navigation by adding onProfileClick handlers to recommendation and story list components. Key changes include updating the UI to support clickable profile elements with hover effects and preventing event bubbling on the subscribe buttons. Review feedback focuses on enhancing web accessibility by adding appropriate ARIA roles and tab indices to clickable containers, ensuring hover styles are only applied when the click handler is present, and maintaining visual consistency across different list element components.
| <div | ||
| className="flex w-full t:w-[296px] min-h-[60px] t:h-[66px] px-[14px] py-[8px] gap-[8px] rounded-[8px] border border-Subbrown-4 bg-white cursor-pointer hover:bg-stone-100 transition-colors group" | ||
| onClick={onProfileClick} | ||
| > |
There was a problem hiding this comment.
onProfileClick이 선택적(optional) 속성이므로, 클릭 가능한 스타일(cursor-pointer, hover 배경색)은 해당 함수가 존재할 때만 적용되는 것이 좋습니다. 또한 div를 클릭 가능하게 만들 때는 웹 접근성을 위해 role="button"과 tabIndex 설정을 권장합니다.
<div
className={`flex w-full t:w-[296px] min-h-[60px] t:h-[66px] px-[14px] py-[8px] gap-[8px] rounded-[8px] border border-Subbrown-4 bg-white transition-colors group ${onProfileClick ? "cursor-pointer hover:bg-stone-100" : ""}`}
onClick={onProfileClick}
role={onProfileClick ? "button" : undefined}
tabIndex={onProfileClick ? 0 : -1}
>
There was a problem hiding this comment.
"타당하고 세심한 리뷰 감사합니다! onProfileClick이 없을 때는 클릭 가능하지 않다는 것을 시각적으로 명확히 전달하도록 조건부 스타일을 적용했습니다. 또한, 웹 접근성 표준을 준수하기 위해 role="button"과 tabIndex 속성을 추가하여 키보드 사용자 및 스크린 리더 사용자도 원활하게 인지하고 이용할 수 있도록 개선하였습니다. 제안 주신 덕분에 컴포넌트의 완성도가 한층 높아졌습니다. 감사합니다!"
| <div | ||
| className="flex w-[296px] h-[66px] px-[14px] py-[8px] gap-[8px] rounded-[8px] border border-Subbrown-4 bg-white cursor-pointer hover:bg-stone-100 transition-colors group" | ||
| onClick={onProfileClick} | ||
| > |
There was a problem hiding this comment.
onProfileClick이 존재할 때만 클릭 가능한 스타일이 적용되도록 조건부 클래스를 사용하는 것이 좋습니다. 또한 웹 접근성을 위해 적절한 ARIA 역할(role)과 탭 인덱스(tabIndex)를 추가하는 것을 권장합니다.
<div
className={`flex w-[296px] h-[66px] px-[14px] py-[8px] gap-[8px] rounded-[8px] border border-Subbrown-4 bg-white transition-colors group ${onProfileClick ? "cursor-pointer hover:bg-stone-100" : ""}`}
onClick={onProfileClick}
role={onProfileClick ? "button" : undefined}
tabIndex={onProfileClick ? 0 : -1}
>
There was a problem hiding this comment.
좋은 지적 감사합니다! list_subscribe_large 컴포넌트 역시 onProfileClick이 옵셔널이므로, 동작이 있을 때만 인터랙션(커서, 호버 배경색)이 발생하도록 조건부 클래스를 적용했습니다. 또한, 웹 접근성 향상을 위해 클릭이 가능한 상태일 때 role="button"과 tabIndex={0}이 활성화되도록 수정하여 일관성 있는 환경을 구축했습니다. 덕분에 더욱 완성도 높은 컴포넌트가 되었습니다!
![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 (5)
src/app/(main)/page.tsx (1)
55-55: LGTM — consider URL-encoding the nickname.Wiring is correct. Minor:
router.push(/profile/${encodeURIComponent(nickname)})would be safer against nicknames containing reserved URL characters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(main)/page.tsx at line 55, The onProfileClick handler currently pushes raw nicknames into the URL via router.push(`/profile/${nickname}`); update the handler used in the component (the onProfileClick prop / arrow function) to URL-encode the nickname before navigation (use encodeURIComponent on the nickname argument) so reserved characters are escaped and routes remain valid.src/components/base-ui/home/Recommendation/list_subscribe_element.tsx (1)
55-55: Duplicate subscribe handler — extract.The exact same
onClick={(e) => { e.stopPropagation(); onSubscribeClick?.(); }}appears on both the tablet and mobile buttons. Extract once:♻️ Proposed refactor
+ const handleSubscribe = (e: React.MouseEvent) => { + e.stopPropagation(); + onSubscribeClick?.(); + }; ... - onClick={(e) => { e.stopPropagation(); onSubscribeClick?.(); }} + onClick={handleSubscribe} ... - onClick={(e) => { e.stopPropagation(); onSubscribeClick?.(); }} + onClick={handleSubscribe}Also applies to: 66-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/base-ui/home/Recommendation/list_subscribe_element.tsx` at line 55, The duplicate inline handler on the tablet and mobile subscribe buttons should be extracted into a single named handler to avoid repetition: create a function (e.g. handleSubscribeClick) inside the component that takes the event, calls e.stopPropagation(), and then invokes onSubscribeClick?.(), then replace both occurrences of onClick={(e) => { e.stopPropagation(); onSubscribeClick?.(); }} with onClick={handleSubscribeClick}; ensure the function is defined in the same scope as the buttons in list_subscribe_element.tsx so it can access onSubscribeClick.src/components/base-ui/home/Story/HomeStoryList.tsx (1)
63-63: Same duplication/encoding note as stories page.
(nickname) => router.push(/profile/${nickname})appears twice here and in several other files in this PR. Consider extracting a shared helper (or a custom hook likeuseProfileNavigation()) and URL-encoding the nickname to defend against special characters.Also applies to: 81-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/base-ui/home/Story/HomeStoryList.tsx` at line 63, The onProfileClick handler (onProfileClick={(nickname) => router.push(`/profile/${nickname}`)}) is duplicated across files; extract a shared helper or custom hook (e.g., useProfileNavigation or navigateToProfile) that takes a nickname, URL-encodes it via encodeURIComponent, and calls router.push(`/profile/${encodedNickname}`), then replace all inline arrow handlers (including the one in HomeStoryList and the duplicate at lines noted) with the new helper to centralize logic and guard against special characters.src/components/base-ui/home/Recommendation/HomeRecommendationSection.tsx (1)
10-10: Prop threading looks good; note type mismatch withListSubscribeElement.
HomeRecommendationSectionProps.onProfileClickis correctly typed as(nickname: string) => voidand invoked withu.nicknameon line 50. However,ListSubscribeElement's prop type isonProfileClick?: () => void(no args) — fine here since the wrapper injects the nickname via closure, but the naming is asymmetric vs.ListSubscribeLargewhich takes a nickname. Worth aligning both child components to the same signature for consistency and to reduce cognitive load.Also applies to: 50-50, 64-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/base-ui/home/Recommendation/HomeRecommendationSection.tsx` at line 10, HomeRecommendationSectionProps defines onProfileClick as (nickname: string) => void but ListSubscribeElement currently types onProfileClick as () => void; make the signatures consistent by changing ListSubscribeElement’s prop type to onProfileClick?: (nickname: string) => void and update its internal invocation (and any wrapper closures) to accept and forward the nickname (the HomeRecommendationSection currently calls onProfileClick(u.nickname) and ListSubscribeLarge already accepts a nickname) so both ListSubscribeElement and ListSubscribeLarge use the same (nickname: string) => void signature.src/app/(main)/stories/page.tsx (1)
172-172: Consider URL-encoding the nickname and deduplicating the handler.
router.push(/profile/${nickname})is duplicated (lines 172 and 181), and rawnicknameis interpolated without encoding. If a nickname contains characters that are not URL-safe (spaces,#,?,/, non-ASCII edge cases), the resulting path can be misinterpreted. Wrap withencodeURIComponent, and extract a single handler to reuse.♻️ Suggested refactor
+ const handleProfileClick = (nickname: string) => + router.push(`/profile/${encodeURIComponent(nickname)}`); ... - onProfileClick={(nickname) => router.push(`/profile/${nickname}`)} + onProfileClick={handleProfileClick} ... - onProfileClick={(nickname) => router.push(`/profile/${nickname}`)} + onProfileClick={handleProfileClick}Also applies to: 181-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(main)/stories/page.tsx at line 172, Extract a single reusable handler (e.g., goToProfile or handleProfileClick) instead of duplicating the inline arrow for onProfileClick, and call router.push from that handler with the nickname URL-encoded using encodeURIComponent (i.e., router.push(`/profile/${encodeURIComponent(nickname)}`)) so nicknames with spaces/special chars are safe; you can memoize it with useCallback if needed and then pass the same handler to both onProfileClick usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/base-ui/home/Recommendation/list_subscribe_element.tsx`:
- Around line 26-29: The card div using onProfileClick (in
list_subscribe_element.tsx) is not keyboard or screen-reader accessible; update
the interactive surface by either wrapping the card with next/link for native
semantics or, if keeping the div, add role="button", tabIndex={0}, an onKeyDown
handler that triggers onProfileClick for Enter and Space, and an aria-label like
`${name} 프로필 보기`; also ensure a visible focus ring via a focus-visible class and
make subscribe button clicks call e.preventDefault() and e.stopPropagation() so
they don't trigger the card navigation.
In `@src/components/base-ui/home/Recommendation/list_subscribe_large.tsx`:
- Around line 28-31: The clickable card currently implemented as a <div> with
only onClick is not keyboard or screen-reader accessible; update the component
(the element using onProfileClick in list_subscribe_large.tsx) to use a semantic
link via next/link when navigating to a profile (e.g., href built from the
profile name) or, if you must keep a non-link container, add role="button",
tabIndex={0}, and an onKeyDown handler that calls onProfileClick when Enter or
Space is pressed; if you switch to Link and still have an inner subscribe button
(onSubscribeClick), ensure the button’s handler calls e.preventDefault() and
e.stopPropagation() to avoid triggering the link navigation.
---
Nitpick comments:
In `@src/app/`(main)/page.tsx:
- Line 55: The onProfileClick handler currently pushes raw nicknames into the
URL via router.push(`/profile/${nickname}`); update the handler used in the
component (the onProfileClick prop / arrow function) to URL-encode the nickname
before navigation (use encodeURIComponent on the nickname argument) so reserved
characters are escaped and routes remain valid.
In `@src/app/`(main)/stories/page.tsx:
- Line 172: Extract a single reusable handler (e.g., goToProfile or
handleProfileClick) instead of duplicating the inline arrow for onProfileClick,
and call router.push from that handler with the nickname URL-encoded using
encodeURIComponent (i.e.,
router.push(`/profile/${encodeURIComponent(nickname)}`)) so nicknames with
spaces/special chars are safe; you can memoize it with useCallback if needed and
then pass the same handler to both onProfileClick usages.
In `@src/components/base-ui/home/Recommendation/HomeRecommendationSection.tsx`:
- Line 10: HomeRecommendationSectionProps defines onProfileClick as (nickname:
string) => void but ListSubscribeElement currently types onProfileClick as () =>
void; make the signatures consistent by changing ListSubscribeElement’s prop
type to onProfileClick?: (nickname: string) => void and update its internal
invocation (and any wrapper closures) to accept and forward the nickname (the
HomeRecommendationSection currently calls onProfileClick(u.nickname) and
ListSubscribeLarge already accepts a nickname) so both ListSubscribeElement and
ListSubscribeLarge use the same (nickname: string) => void signature.
In `@src/components/base-ui/home/Recommendation/list_subscribe_element.tsx`:
- Line 55: The duplicate inline handler on the tablet and mobile subscribe
buttons should be extracted into a single named handler to avoid repetition:
create a function (e.g. handleSubscribeClick) inside the component that takes
the event, calls e.stopPropagation(), and then invokes onSubscribeClick?.(),
then replace both occurrences of onClick={(e) => { e.stopPropagation();
onSubscribeClick?.(); }} with onClick={handleSubscribeClick}; ensure the
function is defined in the same scope as the buttons in
list_subscribe_element.tsx so it can access onSubscribeClick.
In `@src/components/base-ui/home/Story/HomeStoryList.tsx`:
- Line 63: The onProfileClick handler (onProfileClick={(nickname) =>
router.push(`/profile/${nickname}`)}) is duplicated across files; extract a
shared helper or custom hook (e.g., useProfileNavigation or navigateToProfile)
that takes a nickname, URL-encodes it via encodeURIComponent, and calls
router.push(`/profile/${encodedNickname}`), then replace all inline arrow
handlers (including the one in HomeStoryList and the duplicate at lines noted)
with the new helper to centralize logic and guard against special characters.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd847d4e-7efe-4280-8827-2b2c623eec55
📒 Files selected for processing (6)
src/app/(main)/page.tsxsrc/app/(main)/stories/page.tsxsrc/components/base-ui/home/Recommendation/HomeRecommendationSection.tsxsrc/components/base-ui/home/Recommendation/list_subscribe_element.tsxsrc/components/base-ui/home/Recommendation/list_subscribe_large.tsxsrc/components/base-ui/home/Story/HomeStoryList.tsx
| <div | ||
| className="flex w-[296px] h-[66px] px-[14px] py-[8px] gap-[8px] rounded-[8px] border border-Subbrown-4 bg-white cursor-pointer hover:bg-stone-100 transition-colors group" | ||
| onClick={onProfileClick} | ||
| > |
There was a problem hiding this comment.
Accessibility: clickable <div> is not keyboard/screen-reader navigable.
The card is now a primary interactive element but remains a <div> with only onClick. Keyboard users cannot focus or activate it, and assistive tech won't announce it as actionable. Prefer a semantic anchor (ideal for a profile link — enables prefetching, middle-click, "open in new tab", and SEO) or at minimum add role="button", tabIndex={0}, and an onKeyDown handler for Enter/Space.
♻️ Preferred: use `next/link` (requires lifting navigation into the component, or accepting an href prop)
import Link from "next/link";
// ...
<Link
href={`/profile/${encodeURIComponent(name)}`}
className="flex w-[296px] h-[66px] ... cursor-pointer hover:bg-stone-100 ... group"
>
{/* ... */}
<button
type="button"
onClick={(e) => { e.preventDefault(); e.stopPropagation(); onSubscribeClick?.(); }}
...
>
{buttonText}
</button>
</Link>Note: when nesting a <button> inside <a>, use e.preventDefault() in addition to stopPropagation() to suppress the link navigation.
Minimal fallback if you keep the <div>:
<div
className="... cursor-pointer hover:bg-stone-100 ..."
+ role="button"
+ tabIndex={0}
onClick={onProfileClick}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ onProfileClick?.();
+ }
+ }}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/base-ui/home/Recommendation/list_subscribe_large.tsx` around
lines 28 - 31, The clickable card currently implemented as a <div> with only
onClick is not keyboard or screen-reader accessible; update the component (the
element using onProfileClick in list_subscribe_large.tsx) to use a semantic link
via next/link when navigating to a profile (e.g., href built from the profile
name) or, if you must keep a non-link container, add role="button",
tabIndex={0}, and an onKeyDown handler that calls onProfileClick when Enter or
Space is pressed; if you switch to Link and still have an inner subscribe button
(onSubscribeClick), ensure the button’s handler calls e.preventDefault() and
e.stopPropagation() to avoid triggering the link navigation.
There was a problem hiding this comment.
좋은 지적 감사합니다! list_subscribe_large 컴포넌트 역시 onProfileClick이 옵셔널이므로, 동작이 있을 때만 인터랙션(커서, 호버 배경색)이 발생하도록 조건부 클래스를 적용했습니다. 또한, 웹 접근성 향상을 위해 클릭이 가능한 상태일 때 role="button"과 tabIndex={0}이 활성화되도록 수정하여 일관성 있는 환경을 구축했습니다. 덕분에 더욱 완성도 높은 컴포넌트가 되었습니다!
There was a problem hiding this comment.
@shinwokkang 개선 사항 감사합니다! onProfileClick 유무에 따른 조건부 스타일링과 role="button" / tabIndex 적용은 훌륭합니다 👍
한 가지 추가로 말씀드리면, 현재 구현에서는 키보드의 Enter / Space 키로 카드를 활성화할 수 없는 상태입니다. tabIndex={0}으로 포커스는 가능하지만, onKeyDown 핸들러가 없으면 스크린 리더 및 키보드 사용자가 실제로 클릭 동작을 수행할 수 없습니다. 아래와 같이 추가하면 접근성이 완성됩니다:
<div
className={`flex w-[296px] h-[66px] px-[14px] py-[8px] gap-[8px] rounded-[8px] border border-Subbrown-4 bg-white transition-colors group ${onProfileClick ? "cursor-pointer hover:bg-stone-100" : ""}`}
onClick={onProfileClick}
role={onProfileClick ? "button" : undefined}
tabIndex={onProfileClick ? 0 : -1}
onKeyDown={onProfileClick ? (e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
onProfileClick();
}
} : undefined}
>이 부분까지 반영해 주시면 키보드 전용 사용자와 보조 기술(AT) 사용자 모두에게 완전한 접근성을 제공할 수 있습니다! 😊
📌 개요 (Summary)
홈 화면 및 스토리 페이지의 사용자 추천 목록에서 카드 클릭 시 해당 사용자의 프로필 페이지(/profile/[nickname])로 이동하는 기능을 구현했습니다.
관련 이슈: #260
🛠️ 변경 사항 (Changes)
새로운 기능 추가
사용자 추천 카드(컨테이너) 전체를 클릭 가능 영역으로 확대
클릭 시 해당 유저의 프로필로 이동하는 라우팅 로직 연결 (Home, Stories 페이지 공통 적용)
코드 리팩토링
ListSubscribeElement, ListSubscribeLarge 컴포넌트의 인터페이스에 onProfileClick 추가하여 단일 구조 원칙 준수
구독 버튼 클릭 시 프로필 이동이 중복 발생하지 않도록 이벤트 버블링 방지(e.stopPropagation()) 처리
기타 (설명: UX 개선)
마우스 오버 시 시각적 피드백을 위한 hover:bg-stone-100 및 cursor-pointer 스타일링 적용
📸 스크린샷 (Screenshots)
✅ 체크리스트 (Checklist)
상세 구현 노트
이벤트 분리: 카드의 어느 곳을 눌러도 프로필로 이동하지만, '구독' 버튼을 누를 때만은 프로필로 이동하지 않고 순수하게 구독 기능만 동작하도록 설계했습니다.
재사용성: 컴포넌트 내부에서 직접 useRouter를 사용하지 않고 부모로부터 핸들러를 주입받는 방식을 사용하여, 다른 페이지에서 추천 컴포넌트를 재사용할 때도 유연하게 대응할 수 있습니다.
Summary by CodeRabbit