[Feature/#203] 마이페이지 skeleton UI 구현 - #206
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 Walkthrough🔄 전체 변경 사항Setting 페이지에서 API 데이터 로딩 중 스켈레톤 UI를 표시하도록 구현했습니다. 새로운 ProfileSectionSkeleton과 PasswordSectionSkeleton 컴포넌트를 추가하고, Setting 페이지의 isLoading 상태로 로딩 중일 때 스켈레톤을, 로딩 완료 후 실제 데이터를 렌더링하도록 조건부 처리했습니다. 📋 변경 사항스켈레톤 로딩 UI
📝 상세 검토 포인트구조 및 상태 관리
사용성
안정성
🔗 관련 PR
👥 추천 검토자
🎯 2 (Simple) | ⏱️ ~12분 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/setting/Setting.tsx (1)
144-174:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win비동기 작업의 정리(cleanup) 처리 필요
useEffect내의 비동기 함수(fetchMyInfo)가 컴포넌트 언마운트 시 적절히 정리되지 않고 있습니다. 요청이 진행 중에 컴포넌트가 언마운트되면 상태 업데이트 경고(memory leak)가 발생할 수 있어요.
AbortController를 사용해 요청을 취소하는 정리 로직을 추가해주세요. 코드베이스의useClickStream.ts에서도 이 패턴을 사용하고 있으니 참고하면 됩니다.또한 의존성 배열을
[]로 수정하세요.setPreview는 React의useState에서 반환되는 안정적인 참조이므로 매 렌더링마다 동일하며, 현재는 마운트 시에만 데이터를 페칭하는 것이 의도인 것으로 보입니다.🤖 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 `@src/pages/setting/Setting.tsx` around lines 144 - 174, The effect's async fetchMyInfo should be cancellable to avoid state updates after unmount: create an AbortController, pass its signal to getMyInfo (or cancel the underlying request), and check controller.signal.aborted before calling setIsLoading, setDraftProfile, setSavedProfile, and setPreview; add a cleanup that aborts the controller. Also change the useEffect dependency array to [] (setPreview is stable) so fetchMyInfo runs only on mount. Ensure getMyInfo supports/receives the AbortSignal or that you handle cancellation around the fetch promise before calling the setters.
🧹 Nitpick comments (3)
src/components/setting/ProfileSectionSkeleton.tsx (1)
3-41: ⚡ Quick win스켈레톤 컴포넌트에 접근성 속성을 추가해주세요.
현재 스켈레톤 UI가 시각적으로만 로딩 상태를 표시하고 있어요. 스크린 리더 사용자들에게도 로딩 중임을 알릴 수 있도록 적절한 ARIA 속성을 추가하는 것이 좋습니다.
♻️ 접근성 개선 제안
export default function ProfileSectionSkeleton() { return ( - <div className="bg-white border border-gray-100 rounded-component-lg p-8 shadow-Soft"> + <div + className="bg-white border border-gray-100 rounded-component-lg p-8 shadow-Soft" + role="status" + aria-label="프로필 정보 로딩 중" + > <div className="mb-7 flex items-start gap-4">As per coding guidelines: "접근성: 시맨틱 HTML, ARIA 속성 사용 확인."
🤖 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 `@src/components/setting/ProfileSectionSkeleton.tsx` around lines 3 - 41, ProfileSectionSkeleton currently renders only visual skeletons; update the root container in ProfileSectionSkeleton to communicate loading to assistive tech by adding role="status", aria-live="polite", and aria-busy="true" (and an accessible label like aria-label="Loading profile" or an offscreen "Loading profile" text). Mark the presentational Skeleton and SkeletonCircle elements as aria-hidden="true" so screen readers ignore decorative shapes, and ensure the container exposes a single readable loading message for screen readers. Locate these changes in the ProfileSectionSkeleton component and adjust Skeleton/SkeletonCircle usage so they remain decorative while the root status conveys the loading state.src/pages/setting/Setting.tsx (1)
144-174: ⚡ Quick win컴포넌트 언마운트 시 race condition을 방지해주세요.
현재 컴포넌트가 언마운트된 후에도 API 응답이 도착하면
setIsLoading(false)가 실행되어 React에서 경고가 발생할 수 있습니다.AbortController나 cleanup 함수를 사용해서 언마운트 시 상태 업데이트를 방지하는 것이 좋습니다.♻️ cleanup 추가 제안
useEffect(() => { + let isMounted = true; + const fetchMyInfo = async () => { try { setIsLoading(true); const res = await getMyInfo(); const profileData = { name: res.data.name, organizations: res.data.organizations?.map((org) => ({ name: org.orgName, position: org.myRole, })) ?? [], email: res.data.email, phoneNumber: res.data.phoneNumber, }; - setSavedProfile({ - name: res.data.name, - profileImageUrl: res.data.profileImageUrl, - }); - setDraftProfile(profileData); - setPreview(res.data.profileImageUrl); + if (isMounted) { + setSavedProfile({ + name: res.data.name, + profileImageUrl: res.data.profileImageUrl, + }); + setDraftProfile(profileData); + setPreview(res.data.profileImageUrl); + } } catch (error) { - toast.error("회원 정보를 불러오는데 실패했습니다"); + if (isMounted) { + toast.error("회원 정보를 불러오는데 실패했습니다"); + } console.error(error); } finally { - setIsLoading(false); + if (isMounted) { + setIsLoading(false); + } } }; fetchMyInfo(); + + return () => { + isMounted = false; + }; }, [setPreview]);As per coding guidelines: "Hook 사용: useEffect 의존성 배열 및 불필요한 사용 검토."
🤖 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 `@src/pages/setting/Setting.tsx` around lines 144 - 174, The effect's async fetchMyInfo may update state after unmount causing a race; modify useEffect to guard updates by either using an AbortController (pass its signal to getMyInfo if supported) or a mounted flag: inside useEffect create const controller = new AbortController() (or let mounted = true), pass controller.signal to getMyInfo (or check mounted) and in the cleanup call controller.abort() (or set mounted = false); before calling setIsLoading, setSavedProfile, setDraftProfile, setPreview ensure the request wasn't aborted (or mounted is true) and handle abort exceptions silently so no state updates occur after unmount; keep the effect dependencies minimal (remove setPreview from the array if unnecessary).src/components/setting/PasswordSectionSkeleton.tsx (1)
3-30: ⚡ Quick win스켈레톤 컴포넌트에 접근성 속성을 추가해주세요.
ProfileSectionSkeleton과 마찬가지로, 스크린 리더 사용자를 위한 ARIA 속성이 필요합니다.
♻️ 접근성 개선 제안
export default function PasswordSectionSkeleton() { return ( - <div className="bg-white border border-gray-100 rounded-component-lg p-8 shadow-Soft"> + <div + className="bg-white border border-gray-100 rounded-component-lg p-8 shadow-Soft" + role="status" + aria-label="비밀번호 정보 로딩 중" + > <div className="mb-7">As per coding guidelines: "접근성: 시맨틱 HTML, ARIA 속성 사용 확인."
🤖 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 `@src/components/setting/PasswordSectionSkeleton.tsx` around lines 3 - 30, The PasswordSectionSkeleton component lacks ARIA attributes for screen readers; update PasswordSectionSkeleton to wrap the skeleton UI in an accessible status region (e.g., add role="status" and an informative aria-label like "Loading password settings") and include a visually hidden text node (e.g., "Loading password settings") so assistive tech announces the state; also mark purely decorative skeleton pieces (the Skeleton and SkeletonCircle instances) as aria-hidden to avoid noise. Ensure these changes are made inside the PasswordSectionSkeleton function around the top-level container and on the Skeleton/SkeletonCircle elements.
🤖 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.
Outside diff comments:
In `@src/pages/setting/Setting.tsx`:
- Around line 144-174: The effect's async fetchMyInfo should be cancellable to
avoid state updates after unmount: create an AbortController, pass its signal to
getMyInfo (or cancel the underlying request), and check
controller.signal.aborted before calling setIsLoading, setDraftProfile,
setSavedProfile, and setPreview; add a cleanup that aborts the controller. Also
change the useEffect dependency array to [] (setPreview is stable) so
fetchMyInfo runs only on mount. Ensure getMyInfo supports/receives the
AbortSignal or that you handle cancellation around the fetch promise before
calling the setters.
---
Nitpick comments:
In `@src/components/setting/PasswordSectionSkeleton.tsx`:
- Around line 3-30: The PasswordSectionSkeleton component lacks ARIA attributes
for screen readers; update PasswordSectionSkeleton to wrap the skeleton UI in an
accessible status region (e.g., add role="status" and an informative aria-label
like "Loading password settings") and include a visually hidden text node (e.g.,
"Loading password settings") so assistive tech announces the state; also mark
purely decorative skeleton pieces (the Skeleton and SkeletonCircle instances) as
aria-hidden to avoid noise. Ensure these changes are made inside the
PasswordSectionSkeleton function around the top-level container and on the
Skeleton/SkeletonCircle elements.
In `@src/components/setting/ProfileSectionSkeleton.tsx`:
- Around line 3-41: ProfileSectionSkeleton currently renders only visual
skeletons; update the root container in ProfileSectionSkeleton to communicate
loading to assistive tech by adding role="status", aria-live="polite", and
aria-busy="true" (and an accessible label like aria-label="Loading profile" or
an offscreen "Loading profile" text). Mark the presentational Skeleton and
SkeletonCircle elements as aria-hidden="true" so screen readers ignore
decorative shapes, and ensure the container exposes a single readable loading
message for screen readers. Locate these changes in the ProfileSectionSkeleton
component and adjust Skeleton/SkeletonCircle usage so they remain decorative
while the root status conveys the loading state.
In `@src/pages/setting/Setting.tsx`:
- Around line 144-174: The effect's async fetchMyInfo may update state after
unmount causing a race; modify useEffect to guard updates by either using an
AbortController (pass its signal to getMyInfo if supported) or a mounted flag:
inside useEffect create const controller = new AbortController() (or let mounted
= true), pass controller.signal to getMyInfo (or check mounted) and in the
cleanup call controller.abort() (or set mounted = false); before calling
setIsLoading, setSavedProfile, setDraftProfile, setPreview ensure the request
wasn't aborted (or mounted is true) and handle abort exceptions silently so no
state updates occur after unmount; keep the effect dependencies minimal (remove
setPreview from the array if unnecessary).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0540e1e5-623f-4837-abc8-c651517b2cd2
📒 Files selected for processing (3)
src/components/setting/PasswordSectionSkeleton.tsxsrc/components/setting/ProfileSectionSkeleton.tsxsrc/pages/setting/Setting.tsx
🚨 관련 이슈
Closed #203
✨ 변경사항
✏️ 작업 내용
마이페이지(Setting) 페이지 내에 API로딩상태 분기 하고,
ProfileSection과 PasswordSection의 스켈레톤 UI 추가하였습니다
💻 작업 화면
😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
Summary by CodeRabbit
릴리스 노트