[Feat/#64] 마이페이지 중앙 정렬 요청 및 solve 이미지 배열 #65
Conversation
WalkthroughMultiple CSS files are updated with standardized flex-based layouts, unified width constraints (66.8rem), and centering properties across pages. ReviewNotes.tsx is refactored to wrap content in a new div, with corresponding styles exported in its CSS file. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (1)src/pages/reviewNoteDetail/reviewNoteDetail.css.ts (1)
🔇 Additional comments (3)
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: 0
🧹 Nitpick comments (6)
src/pages/my/my.css.ts (2)
10-12: Centering looks good; ensure full-viewport background on mobile.Add a minHeight to avoid short pages showing partial gradient.
export const container = style({ paddingTop: '9.6rem', height: '100%', + minHeight: '100dvh', background: 'linear-gradient(355deg, #FFF 48.23%, #BFD9FE 97.05%), #FFF', backgroundRepeat: 'no-repeat', display: 'flex', flexDirection: 'column', alignItems: 'center', });
19-21: Prevent horizontal overflow from width:100% + padding.Use border-box so padding doesn’t exceed the container width; keeps centering crisp.
export const title = style({ display: 'flex', flexDirection: 'row', paddingLeft: '3.6rem', width: '100%', maxWidth: '66.8rem', + boxSizing: 'border-box', }); export const noteDiv = style({ padding: '1.2rem 2.6rem 2.6rem', borderRadius: '12px', width: '100%', maxWidth: '66.8rem', + boxSizing: 'border-box', }); export const buttonContainer = style({ display: 'flex', flexDirection: 'column', gap: '1.6rem', padding: '3.6rem 3.6rem 10rem', maxWidth: '66.8rem', width: '100%', + boxSizing: 'border-box', });Also applies to: 45-46, 192-194
src/pages/solve/Solve.tsx (4)
249-254: Use const and limit scope of finalUploadedUrls.No need for a mutable outer variable; declare it where resolved.
- let finalUploadedUrls: string[] = []; ... - finalUploadedUrls = await Promise.all(uploadPromises); + const finalUploadedUrls = await Promise.all(uploadPromises); ... - setDownloadUrls(finalUploadedUrls); + setDownloadUrls(finalUploadedUrls);Also applies to: 283-285, 299-300
262-281: Type the promises, remove redundant ok-check, and simplify with map.Cleaner, safer, and preserves order without manual push/for.
- // Promise 배열을 filesArray의 순서대로 정의 - const uploadPromises = []; - - // S3 업로드 루프: filesArray의 원래 순서(사용자 선택 순서로 추정)대로 진행 - for (let i = 0; i < expectedCount; i++) { - const fileToUpload = filesArray[i]!; // 원래 순서의 파일 - const uploadUrl = uploadUrls[i]; - const downloadUrl = presignedUrls[i]; // 해당 순서의 다운로드 URL - - // Promise는 HTTP 요청만 담고, 결과를 해당 순서의 downloadUrl로 resolve - // Promise.all은 입력된 배열 순서대로 결과를 반환하므로, filesArray의 순서가 유지 - uploadPromises.push( - uploadToPresignedUrl(uploadUrl, fileToUpload).then((response) => { - if (!response.ok) { - throw new Error('S3 업로드 실패'); - } - return downloadUrl; // filesArray[i]에 해당하는 URL 반환 - }), - ); - } + // 서버에서 전달된 배열 길이 검증 + if ( + uploadUrls.length !== expectedCount || + presignedUrls.length !== expectedCount + ) { + throw new Error('서버로부터 받은 URL 수가 파일 수와 일치하지 않습니다.'); + } + + // filesArray 순서를 그대로 보장 + const uploadPromises: Promise<string>[] = filesArray.map((file, i) => + uploadToPresignedUrl(uploadUrls[i]!, file).then(() => presignedUrls[i]!), + );
287-301: Differentiate upload vs preload failures for clearer UX.Show a specific message when image preloading fails.
- await preloadImages(finalUploadedUrls); + await preloadImages(finalUploadedUrls); ... - } catch { - // 6. 실패 시 - addServerMessage( - '이미지 업로드 중 오류가 발생했습니다. 다시 시도해 주세요.', - ); + } catch (err) { + const msg = + err instanceof Error ? err.message : String(err ?? ''); + addServerMessage( + msg.startsWith('Failed to load image') + ? '이미지를 불러오는 중 오류가 발생했습니다. 다시 시도해 주세요.' + : '이미지 업로드 중 오류가 발생했습니다. 다시 시도해 주세요.', + ); // 실패해도 로딩 상태는 초기화 setUploadingSlots([]); setIsUploading(false);Also applies to: 301-309
142-170: Minor: returned cleanup in event handler is dead code.The function’s return value is ignored; use a ref to manage timeouts if cleanup is needed.
- const handleStepByStep = () => { + const handleStepByStep = () => { setIsPending(true); const timeoutId = setTimeout(() => { const finished = showNextStep(setChatList); ... setIsPending(false); }, 500); - return () => clearTimeout(timeoutId); + // If cancellation is needed, store in a ref and clear on demand. + // stepTimeoutRef.current = timeoutId; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/my/my.css.ts(3 hunks)src/pages/solve/Solve.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/solve/Solve.tsx (1)
src/shared/apis/upload.ts (1)
uploadToPresignedUrl(1-15)
🔇 Additional comments (2)
src/pages/solve/Solve.tsx (2)
235-244: Early count validation LGTM.Good UX to close modal then validate and reset input on mismatch.
Please verify messages for both one/two-image flows still guide users clearly about the expected selection.
250-270: Root-cause check: does Windows preserve the actual selection order?You now trust FileList order. On Windows, some browsers sort by name/path on multi-select. Please verify across Chrome/Edge.
Manual QA:
- Multi-select 2 files by Control-click in different orders.
- Repeat by Shift-select (contiguous) and drag-and-drop if supported.
- Confirm Chat image order matches selection order in Windows 10/11 (Chrome, Edge).
If Windows still reorders, we’ll need an explicit order heuristic (e.g., timestamp firstSelectedAt via oninput, or compare lastModified if available).
|
🎩 빌드에 성공했습니다! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/solve/Solve.tsx (1)
259-272: Sequential uploads fix ordering but impact performance.The sequential upload approach (awaiting each upload individually) guarantees deterministic ordering and solves the Windows issue. However, this doubles the upload time for two images compared to parallel uploads.
Note: The non-null assertions (
filesArray[0]!,filesArray[1]!) are safe here because the validation at line 236 confirms the correct file count.Consider this parallel alternative that maintains ordering while improving performance:
- // [최종 순서 보장 로직] expectedCount가 2일 경우, 파일을 순차적으로 처리 if (expectedCount === 2) { - // 1) 첫 번째 파일(filesArray[0]) 처리 (문제 이미지로 추정) - await uploadToPresignedUrl(uploadUrls[0], filesArray[0]!); - finalUploadedUrls.push(presignedUrls[0]); - - // 2) 두 번째 파일(filesArray[1]) 처리 (풀이 이미지로 추정) - await uploadToPresignedUrl(uploadUrls[1], filesArray[1]!); - finalUploadedUrls.push(presignedUrls[1]); + // Upload both files in parallel but maintain order + await Promise.all([ + uploadToPresignedUrl(uploadUrls[0], filesArray[0]!), + uploadToPresignedUrl(uploadUrls[1], filesArray[1]!) + ]); + finalUploadedUrls.push(presignedUrls[0], presignedUrls[1]); } else {This maintains the guaranteed ordering (since
finalUploadedUrlsis populated in the correct sequence) while allowing concurrent uploads.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/solve/Solve.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/solve/Solve.tsx (1)
src/shared/apis/upload.ts (1)
uploadToPresignedUrl(1-15)
🔇 Additional comments (4)
src/pages/solve/Solve.tsx (4)
235-244: LGTM! Clear validation logic.The file count validation is properly implemented with user-friendly error messages and appropriate input cleanup.
246-250: Good initialization for ordered processing.The
filesArrayandfinalUploadedUrlsapproach provides explicit control over the upload and display order, which addresses the Windows ordering issue mentioned in issue #64.
275-285: Excellent ordering preservation through the entire flow.The use of
finalUploadedUrlsconsistently throughout preloading and image addition ensures the deterministic display order. The preloading step also improves UX by preventing image loading flashes.
287-289: Frontend correctly maintains array ordering; backend handling requires manual verification.The frontend code properly enforces array ordering: problem image at index 0, solution image at index 1 (lines 263-271). The
downloadUrlsarray is passed directly to the API as a JSON payload. However, verification that the backend API endpoint respects and correctly processes this array ordering should be confirmed with the backend team or API documentation, as this cannot be determined from the codebase alone.
|
🎩 빌드에 성공했습니다! |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/pages/solve/Solve.tsx (3)
228-230: Remove duplicate comment lineTwo identical “파일 선택 핸들러” comments back-to-back. Drop one for cleanliness.
- // 파일 선택 핸들러 - // 파일 선택 핸들러 + // 파일 선택 핸들러
232-243: Prefer e.currentTarget over e.target; simplify value resetsUse the strongly typed currentTarget to avoid accidental target mismatches and duplicate resets.
- const fileList = e.target.files; + const input = e.currentTarget; + const fileList = input.files; @@ - e.target.value = ''; + input.value = ''; return;Also update the final reset below (Line 285) to use input.value.
- e.target.value = ''; + input.value = '';
260-264: Optional: keep order but upload in parallel to cut latencyCurrent sequential loop enforces order but increases total time. You can upload in parallel and still preserve display order by indexing.
- const finalUploadedUrls: string[] = []; - - // ✅ 순서대로 업로드 (문제 → 풀이 이미지) - for (let i = 0; i < expectedCount; i++) { - await uploadToPresignedUrl(uploadUrls[i], filesArray[i]!); - finalUploadedUrls.push(presignedUrls[i]); - } + await Promise.all( + filesArray.map((file, i) => uploadToPresignedUrl(uploadUrls[i], file!)), + ); + const finalUploadedUrls = presignedUrls.slice();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/solve/Solve.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/solve/Solve.tsx (1)
src/shared/apis/upload.ts (1)
uploadToPresignedUrl(1-15)
🔇 Additional comments (1)
src/pages/solve/Solve.tsx (1)
272-277: LGTM: display and downstream state preserve selection orderDisplaying via finalUploadedUrls and updating downloadUrls accordingly keeps UI/AI input aligned.
src/pages/solve/Solve.tsx
Outdated
| // 이미지 프리로딩 | ||
| await preloadImages(finalUploadedUrls); | ||
|
|
There was a problem hiding this comment.
Don’t treat preload failures as upload failures; handle separately
A failed image decode/load will hit the catch and show “이미지 업로드 중 오류…”, even though uploads succeeded. Handle preload errors separately so users still see uploaded images.
- // 이미지 프리로딩
- await preloadImages(finalUploadedUrls);
+ // 이미지 프리로딩 (실패해도 업로드는 성공으로 처리)
+ try {
+ await preloadImages(finalUploadedUrls);
+ } catch (err) {
+ console.warn('Image preload failed; proceeding to display anyway.', err);
+ }
@@
- } catch {
+ } catch {
addServerMessage(
- '이미지 업로드 중 오류가 발생했습니다. 다시 시도해주세요.',
+ '이미지 업로드 중 오류가 발생했습니다. 다시 시도해주세요.',
);(Optional: if you want user-facing distinction, emit a softer “이미지 미리보기 준비에 실패했지만 업로드는 완료되었습니다.” on preload errors.)
Also applies to: 279-284
🤖 Prompt for AI Agents
In src/pages/solve/Solve.tsx around lines 266-268 (and similarly 279-284), the
current await preloadImages(...) is inside the main try that treats any preload
failure as an upload error; change it so preloadImages errors are handled
separately: call preloadImages in its own try/catch (or attach .catch) so upload
success path is not considered a failure if preloading fails, log the preload
error and optionally show a softer user-facing message like "이미지 미리보기 준비에 실패했지만
업로드는 완료되었습니다." without throwing or returning from the outer upload success flow.
|
🎩 빌드에 성공했습니다! |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/solve/Solve.tsx (1)
150-178: Returning a cleanup function from an event handler is a no‑op.The returned function isn’t used; just schedule the timeout, or manage a ref if you need cancellation.
- const timeoutId = setTimeout(() => { + setTimeout(() => { const finished = showNextStep(setChatList); @@ // pending 종료 setIsPending(false); }, 500); - return () => clearTimeout(timeoutId);
♻️ Duplicate comments (1)
src/pages/solve/Solve.tsx (1)
291-298: Don’t fail the whole upload if image preload fails.Preload errors currently trigger the outer catch and show “업로드 오류”. Treat preload as best‑effort so successful uploads aren’t misreported. (This mirrors a prior comment.)
- // 4. S3 업로드 완료 후, 프리로딩 시작 - await preloadImages([downloadUrlToUse]); + // 4. S3 업로드 완료 후, 프리로딩 시작 (실패해도 업로드는 성공 처리) + try { + await preloadImages([downloadUrlToUse]); + } catch (err) { + console.warn('Image preload failed; proceeding anyway.', err); + // (옵션) 사용자 안내: addServerMessage('이미지 미리보기 준비에 실패했지만 업로드는 완료되었습니다.'); + }
🧹 Nitpick comments (4)
src/pages/solve/Solve.tsx (4)
308-315: Programmatic file input click may be blocked by user-activation; add a fallback cue.setTimeout breaks user gesture; some UAs will block the dialog. Add a visible cue so users know to tap the camera if the dialog doesn’t open.
setTimeout(() => { // setTimeout이 User Activation 오류의 원인이지만, UX를 위해 일단 유지 if (fileInputRef.current) { + // 대체 UX 신호: 다이얼로그가 막혀도 사용자가 다음을 이해하도록 + addServerMessage('이제 풀이 이미지를 선택해 주세요. 다이얼로그가 뜨지 않으면 카메라를 다시 눌러 주세요.'); fileInputRef.current.multiple = false; fileInputRef.current.click(); } }, 300);
254-259: Add basic client-side validation (type/size) before upload.Prevents wasted bandwidth and clearer errors.
const file = files[0]; + // 파일 유효성 검사 + const MAX_MB = 10; + if (!file.type.startsWith('image/')) { + addServerMessage('이미지 파일만 업로드할 수 있습니다.'); + e.target.value = ''; + return; + } + if (file.size > MAX_MB * 1024 * 1024) { + addServerMessage(`이미지 용량은 최대 ${MAX_MB}MB까지 지원합니다.`); + e.target.value = ''; + return; + }
288-290: Optional: add an upload timeout/cancel path.Network stalls will hang indefinitely. Consider extending uploadToPresignedUrl to accept AbortSignal and applying a 30–60s timeout here.
Example (in shared uploader):
export const uploadToPresignedUrl = async (uploadUrl: string, file: File|Blob, opts?: { signal?: AbortSignal }) => { const res = await fetch(uploadUrl, { method: 'PUT', headers: { 'Content-Type': file.type }, body: file, signal: opts?.signal }); if (!res.ok) throw new Error(`S3 upload failed: ${res.status} ${res.statusText}`); };Usage here:
const ctrl = new AbortController(); const t = setTimeout(() => ctrl.abort(), 60000); await uploadToPresignedUrl(uploadUrlToUse, file, { signal: ctrl.signal }); clearTimeout(t);
335-343: Step reset on failure is OK; consider re-issuing presigned URLs after repeated failures.If a presigned URL expires during retries, auto-refreshing URLs when currentUploadStep remains unchanged would harden the flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/solve/Solve.tsx(3 hunks)src/pages/solve/solve.css.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/pages/solve/solve.css.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/solve/Solve.tsx (3)
src/pages/solve/apis/axios.ts (1)
getPresignedUrl(6-15)src/shared/apis/upload.ts (1)
uploadToPresignedUrl(1-15)src/pages/solve/ChatLogic.ts (1)
solutionStepsRef(11-14)
🔇 Additional comments (1)
src/pages/solve/Solve.tsx (1)
243-252: Two-step guided picker removes Windows FileList-order risk — nice.Enforcing one file per step plus prefetching presigned URLs resolves the “Windows returns alphabetical order” issue and aligns with previous feedback. Please sanity-check on Windows (Chrome/Edge) that step 2 is reachable and URLs persist after modal close.
Also applies to: 265-280, 371-387
|
🎩 빌드에 성공했습니다! |
|
🎩 빌드에 성공했습니다! |
📌 Related Issues
✅ 체크 리스트
📄 Tasks
📷 Screenshot
🔔 ETC
Summary by CodeRabbit