-
Notifications
You must be signed in to change notification settings - Fork 1
[25.06.07 / TASK-189] Refactor - QR 로그인 모달 UI 일부 개선 #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
""" Walkthrough이 변경사항은 QRCode 컴포넌트에 만료 타이머, 만료 시 시각적 표시, 새로고침 기능, 클립보드 복사 버튼 등을 추가하여 QR 코드의 상태와 상호작용을 확장하였습니다. 내부 상태 관리와 UI 피드백이 강화되었으며, React 훅과 쿼리 옵션이 업데이트되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QRCodeComponent
participant API
User->>QRCodeComponent: 컴포넌트 마운트
QRCodeComponent->>API: QR 코드 토큰 요청
API-->>QRCodeComponent: 토큰 및 URL 응답
QRCodeComponent->>QRCodeComponent: 타이머(5분) 시작
loop 타이머 카운트다운
QRCodeComponent->>QRCodeComponent: 1초마다 timeLeft 감소
end
QRCodeComponent-->>User: QR 코드 및 남은 시간 표시
User->>QRCodeComponent: 복사 버튼 클릭
QRCodeComponent->>User: 클립보드 복사 완료 피드백
alt 만료 시
QRCodeComponent-->>User: 만료 표시, 새로고침 버튼 노출
User->>QRCodeComponent: 새로고침 버튼 클릭
QRCodeComponent->>API: QR 코드 토큰 재요청
API-->>QRCodeComponent: 새 토큰 응답
QRCodeComponent->>QRCodeComponent: 타이머 재시작
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
""" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/(auth-required)/components/QRCode.tsx (1)
111-129
: 접근성 개선을 위한 aria-label 추가 권장버튼의 상태와 기능을 스크린 리더 사용자에게 명확히 전달하기 위해 aria-label을 추가하는 것이 좋습니다.
<button onClick={handleClick} disabled={isExpired} + aria-label={isExpired ? 'QR 코드가 만료되었습니다' : clicked ? 'URL이 복사되었습니다' : 'URL을 클립보드에 복사하려면 클릭하세요'} className={`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/(auth-required)/components/QRCode.tsx
(1 hunks)
🔇 Additional comments (1)
src/app/(auth-required)/components/QRCode.tsx (1)
14-31
: QR 로그인 모달 개선사항이 잘 구현되었습니다만료 시간 표시, URL 복사 기능, 자동 새로고침 등 PR 목표에 맞는 기능들이 적절히 구현되었습니다. React Query 옵션 설정도 QR 코드 사용 사례에 적합합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아쉬운 점
음, 아주 솔직히 너무 급하게 하신 것 같아요.
- QRCode 덩어리가 너무 커졌어요.
formatTime
은 충분히 여기 외 다른 곳에 있어도 괜찮을 것 같아요. 특히handleClick
도 역시요. 모두 utils 등에 들어가도 무방할 것 같아요. - setInterval 이 사실 클릭할때마다 계속 clear 되고 재성성 되는 구조에요.
- isSuccess 상태와 isExpired 상태 복합 랜더링인데, 차라리 상수를 하나 분리하던지, 원래 기준님이 하시던 방식대로 내부 components 디렉토리에 하나 추가하던지가 훨씬 낫지 않을까요?
좋았던 점
그래도 타이머가 깔끔하고, 최소한 중첩타이머 등의 이슈로 메모리는 안먹어서 디테일이 (clear) 느껴졌어요!
UI 가 이뻐요! 확실히 기준님이 깔끔한 화면에 감각이 있으신 듯
말씀해주신대로 분리할 부분은 분리하고, setInterval도 조금 정리해봤습니다! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/app/(auth-required)/components/qrcode/CopyButton.tsx (3)
5-8
: 인터페이스 명명 개선 제안
IProp
대신 더 구체적인 이름을 사용하는 것이 좋겠습니다.-interface IProp { +interface CopyButtonProps { url?: string; disabled?: boolean; }그리고 컴포넌트에서도 해당 타입을 사용하도록 수정:
-export const CopyButton = ({ url, disabled }: IProp) => { +export const CopyButton = ({ url, disabled }: CopyButtonProps) => {
14-27
: 클립보드 API 브라우저 호환성 및 에러 처리 개선현재 구현은 적절하지만, 다음 사항들을 고려해보세요:
navigator.clipboard
API의 브라우저 호환성- 에러 처리 시 사용자 피드백 부족
const handleClick = async () => { if (clicked || !url) return; + if (!navigator.clipboard) { + console.error('클립보드 API가 지원되지 않습니다.'); + return; + } try { await navigator.clipboard.writeText(url); setClicked(true); if (clickedRef.current) clearTimeout(clickedRef.current); clickedRef.current = setTimeout(() => setClicked(false), ROLLBACK_AFTER_CLICK_MS); } catch (err) { console.error('클립보드 복사 실패:', err); + // 선택적: 사용자에게 피드백 제공 (토스트 메시지 등) } };
30-47
: 접근성 개선 및 스타일링 최적화 제안버튼의 접근성과 코드 가독성을 개선할 수 있습니다.
<button onClick={handleClick} disabled={disabled} + aria-label={clicked ? '복사 완료' : '클릭해서 복사하기'} + title={url} className={` relative block p-4 rounded-lg leading-none overflow-hidden transition-all duration-200 after:absolute after:inset-0 after:flex after:items-center after:justify-center truncate after:rounded-lg after:transition-all after:duration-300 after:font-medium after:pointer-events-none ${ disabled ? 'cursor-not-allowed bg-BG-ALT text-TEXT-SUB opacity-50' : clicked ? 'cursor-pointer bg-BG-MAIN text-TEXT-MAIN hover:shadow-lg after:content-["복사_완료!"] after:bg-PRIMARY-SUB after:text-BG-MAIN after:opacity-100 after:scale-100' : 'cursor-pointer bg-BG-MAIN text-TEXT-MAIN hover:shadow-lg after:content-["클릭해서_복사하기"] after:bg-BG-MAIN after:text-TEXT-MAIN after:opacity-0 after:scale-95 hover:after:opacity-100 hover:after:scale-100' } `} >또는 스타일 로직을 분리하여 가독성을 개선할 수 있습니다:
+const getButtonStyles = (disabled: boolean, clicked: boolean) => { + const baseStyles = 'relative block p-4 rounded-lg leading-none overflow-hidden transition-all duration-200 after:absolute after:inset-0 after:flex after:items-center after:justify-center truncate after:rounded-lg after:transition-all after:duration-300 after:font-medium after:pointer-events-none'; + + if (disabled) return `${baseStyles} cursor-not-allowed bg-BG-ALT text-TEXT-SUB opacity-50`; + if (clicked) return `${baseStyles} cursor-pointer bg-BG-MAIN text-TEXT-MAIN hover:shadow-lg after:content-["복사_완료!"] after:bg-PRIMARY-SUB after:text-BG-MAIN after:opacity-100 after:scale-100`; + return `${baseStyles} cursor-pointer bg-BG-MAIN text-TEXT-MAIN hover:shadow-lg after:content-["클릭해서_복사하기"] after:bg-BG-MAIN after:text-TEXT-MAIN after:opacity-0 after:scale-95 hover:after:opacity-100 hover:after:scale-100`; +};src/app/(auth-required)/components/qrcode/index.tsx (2)
44-57
: 조건부 렌더링 로직 단순화 제안현재 구현은 작동하지만, PR 코멘트에서 언급된 대로 복잡한 렌더링 로직을 단순화할 수 있습니다.
상태를 관리하는 상수를 도입하거나 별도 컴포넌트로 분리할 수 있습니다:
+const getQRState = (isExpired: boolean, isLoading: boolean) => { + if (isLoading) return { overlay: '로딩중', shouldBlur: true }; + if (isExpired) return { overlay: '만료됨', shouldBlur: true }; + return { overlay: null, shouldBlur: false }; +}; export const QRCode = () => { // ... existing code ... + const qrState = getQRState(isExpired, isLoading); return ( <Layout title="QR 로그인"> <div className="flex items-center justify-center gap-10"> <div - className={`relative ${isExpired || isLoading ? `after:inset-0 ${isLoading ? 'after:content-["로딩중"]' : 'after:content-["만료됨"]'} after:absolute after:m-auto after:bg-BG-MAIN after:size-fit after:text-TEXT-MAIN after:px-3 after:py-1 after:rounded-lg after:font-medium` : ''}`} + className={`relative ${qrState.shouldBlur ? `after:inset-0 after:content-["${qrState.overlay}"] after:absolute after:m-auto after:bg-BG-MAIN after:size-fit after:text-TEXT-MAIN after:px-3 after:py-1 after:rounded-lg after:font-medium` : ''}`} > <QRCodeSVG value={url} width={width < SCREENS.MBI ? 130 : 171} height={width < SCREENS.MBI ? 130 : 171} enableBackground={0} bgColor={COLORS.BG.SUB} fgColor={COLORS.TEXT.MAIN} - className={`transition-all ${isExpired || isLoading ? 'blur-sm' : ''}`} + className={`transition-all ${qrState.shouldBlur ? 'blur-sm' : ''}`} /> </div>
29-29
: URL 생성 최적화URL 생성을 메모이제이션하여 불필요한 재계산을 방지할 수 있습니다.
+import { useState, useRef, useEffect, useMemo } from 'react'; export const QRCode = () => { // ... existing code ... - const url = `${env.BASE_URL}/api/qr-login?token=${data?.token}`; + const url = useMemo(() => + data?.token ? `${env.BASE_URL}/api/qr-login?token=${data.token}` : '', + [data?.token] + ); // ... rest of component ... <CopyButton url={url} disabled={isExpired || isLoading} />이렇게 하면 토큰이 변경될 때만 URL이 재생성됩니다.
Also applies to: 79-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/(auth-required)/components/QRCode.tsx
(0 hunks)src/app/(auth-required)/components/header/index.tsx
(1 hunks)src/app/(auth-required)/components/index.ts
(1 hunks)src/app/(auth-required)/components/qrcode/CopyButton.tsx
(1 hunks)src/app/(auth-required)/components/qrcode/index.tsx
(1 hunks)src/utils/dateUtil.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- src/app/(auth-required)/components/QRCode.tsx
✅ Files skipped from review due to trivial changes (3)
- src/app/(auth-required)/components/header/index.tsx
- src/app/(auth-required)/components/index.ts
- src/utils/dateUtil.ts
🔇 Additional comments (1)
src/app/(auth-required)/components/qrcode/index.tsx (1)
22-28
: 쿼리 설정이 적절합니다QR 토큰의 특성상 항상 최신 데이터가 필요하므로 현재 설정이 적절합니다:
refetchOnMount: true
: 컴포넌트 마운트 시 새로운 토큰 요청staleTime: 0
: 데이터를 즉시 stale로 처리하여 신선한 토큰 보장refetchOnWindowFocus: false
: 불필요한 재요청 방지
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 잘 봤습니다! 저는 이 타이머 재생성 문제만 해결되면 괜찮을 것 같네요~!
고생하셨습니다! 👍 🔥
좋았던 점
- UI가 깔끔하고 예뻐요!! 디테일이 👍👍
00분 00초
포맷 관련 유틸로 빼신 부분 좋았습니다.padStart
로 두자리 맞춰주시는 것도요!- 컴포넌트도 적절히 분리된 것 같고 코드도 깔끔해서 읽기 좋았습니다~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용자 입장에서 필요한 요소들이 직관적으로 드러나는 UI인 것 같습니다!👏 고생하셨습니다~!
좋았던 점
- CopyButton을 별도 파일로 분리해 재사용성이 좋아진 것 같습니다!
- 클립보드 복사 후 UI 피드백(after:content) 처리도 깔끔하고 사용자 입장에서 직관적인 것 같습니다~!
- QR 재발급 시 타이머 리셋 처리도 꼼꼼하게 잘 해주신 것 같아요!!
🔥 변경 사항
QR 로그인 모달 UI를 일부 개선하였습니다!
만료 시간과 URL 복사 버튼을 추가하고, 창을 다시 열면 자동 새로고침되도록 수정하였습니다!
🏷 관련 이슈
#189
📸 스크린샷 (UI 변경 시 필수)
📌 체크리스트
Summary by CodeRabbit