Skip to content

Conversation

@hansoojeongsj
Copy link
Member

@hansoojeongsj hansoojeongsj commented Oct 27, 2025

πŸ“Œ Related Issues

βœ… 체크 리슀트

  • PR 제λͺ©μ˜ ν˜•μ‹μ„ 잘 μž‘μ„±ν–ˆλ‚˜μš”? e.g. [Feat] PR ν…œν”Œλ¦Ώ μž‘μ„±
  • λΉŒλ“œκ°€ μ„±κ³΅ν–ˆλ‚˜μš”? (pnpm build)
  • μ»¨λ²€μ…˜μ„ μ§€μΌ°λ‚˜μš”?

πŸ“„ Tasks

μ„œλ²„ 열리고 잘 μž‘λ™ν•˜λŠ”μ§€ 확인 ν›„ λ¨Έμ§€ μ˜ˆμ •

πŸ“· Screenshot

πŸ”” ETC

Summary by CodeRabbit

  • New Features

    • Concurrent image upload/preload with a visual uploading indicator; delayed guidance message after flows
    • Back button in header and refined home/logo behavior
    • Root-level error boundary and catch-all error page
  • Bug Fixes

    • Prevented duplicate login callback execution
    • Improved upload error handling and recovery; input disabled during uploads
    • Better handling of network/server-down errors
  • Style

    • Increased max layout width, spacing, heights and responsive sizing; adjusted scroll/animation thresholds

@hansoojeongsj hansoojeongsj linked an issue Oct 27, 2025 that may be closed by this pull request
4 tasks
@github-actions
Copy link

🎩 λΉŒλ“œμ— μ„±κ³΅ν–ˆμŠ΅λ‹ˆλ‹€!

@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2025

Walkthrough

Adjusts multiple layout breakpoints, container sizes, and animation timings on home; adds image preloading and an uploading state/UI to the solve page; prevents repeated OAuth callback execution; increases footer height and global max-width; adds network-down redirect handling in API interceptor; wraps the home route with a global error boundary and moves the root-level catch-all route.

Changes

Cohort / File(s) Summary
Home layout & global max-width
src/shared/styles/global.css.ts, src/pages/home/home.css.ts
Increased global max-width to 1180px; centered/constrained floatingSolveBtn container (width, maxWidth, left, transform, paddingRight, boxSizing, zIndex).
SectionTop layout
src/pages/home/components/sectionTop/SectionTop.tsx, src/pages/home/components/sectionTop/sectionTop.css.ts
Increased group maxWidth and right padding; replaced fixed IcMainGroup width/height with responsive clamp(...) width and height: auto; added willChange: 'opacity'; increased sectionTopWrapper height.
Scroll & SectionBottom animations
src/pages/home/components/scrollText/ScrollText.tsx, src/pages/home/components/sectionBottom/SectionBottom.tsx, src/pages/home/components/sectionBottom/sectionBottom.css.ts
Changed BREAKPOINT from 2480 β†’ 2400, shifting position/opacity thresholds; adjusted FADE_IN_START, FADE_IN_GAP, FADE_OUT_GAP, and translateYIn mapping; increased sectionBottomWrapper height.
Solve page β€” upload flow & styles
src/pages/solve/Solve.tsx, src/pages/solve/solve.css.ts
Added preloadImages(urls) and isUploading/uploadingSlots state; uploads now follow user-selected order, show a three-dot uploading indicator, preload images, append uploaded URLs to chat with delayed messages, and block input during uploading; added chatMyText style and dot-color override inside chatBubbleRight.
Auth callback guard
src/pages/loginCallback/LoginCallback.tsx
Added hasRunRef (useRef) to prevent the login callback effect from running multiple times.
Routing & error boundary
src/routes/router.tsx
Wrapped HOME route element with GlobalErrorBoundary; moved catch-all ('*') route to the root level for global 404 handling.
API interceptor network handling
src/shared/apis/interceptor.ts
On absent error.response (network/server down) navigate to /error and reject early, before other error flows.
Header & icons
src/shared/components/header/Header.tsx, src/shared/components/header/header.css.ts, src/shared/components/icons/IcLeftArrow.tsx, src/shared/components/icons/index.ts
Added back navigation icon/component (IcLeftArrow), conditional back button and left/right group layout in header, new leftGroup/rightGroup styles, and exported IcLeftArrow.
Footer & My page styles
src/shared/components/footer/footer.css.ts, src/pages/my/my.css.ts
Increased footer wrapper height to 18rem; constrained chipListWrapper with width: '100%' and maxWidth: '60rem'.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant Modal as File Input / Modal
    participant UploadMgr as Upload Manager
    participant Preloader
    participant Chat

    User->>Modal: Select file(s)
    Modal->>UploadMgr: Close modal, start uploading (isUploading = true)
    UploadMgr->>UploadMgr: Request presigned URLs, upload files in user-selected order
    UploadMgr->>Preloader: preloadImages(urls) (concurrent)
    Preloader-->>UploadMgr: All images loaded (or error)
    alt success
        UploadMgr->>Chat: Add uploaded image URLs to chat (with delayed guidance message)
    else failure
        UploadMgr-->>Chat: Show upload error message
    end
    UploadMgr->>UploadMgr: Clear uploading state, reset file input
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review Solve.tsx carefully: concurrency in preloadImages, isUploading lifecycle, upload error and reset paths.
  • Check router changes: GlobalErrorBoundary wrapping and moved root-level catch-all for unintended route prop/behavior changes.
  • Verify interceptor network-down navigation doesn't conflict with existing auth/401 flows.

Possibly related PRs

Suggested labels

⭐ Feature

Poem

🐰 I nibble breakpoints, stretch the view,
I preload pixels so images queue,
Callbacks behave, uploads flash three,
Footers grow tall, headers back for me,
Hop β€” code and carrot, snug and new! πŸ₯•

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The PR title "[Refactor/#60] self qa" references the linked issue and indicates this is a refactoring for quality assurance purposes. However, the title is vague and genericβ€”it references the issue number and mentions "self qa" but does not clearly convey what the primary changes or improvements are. A teammate scanning the commit history would not immediately understand the scope of the work from this title alone. While the title is related to the PR's intent and not misleading, it lacks specificity about the actual changes made.
Out of Scope Changes Check ❓ Inconclusive The PR includes numerous changes that extend beyond the four specific objectives listed in issue #60, including: extensive CSS refinements across multiple components (ScrollText breakpoint adjustment, SectionTop/SectionBottom animations and sizing), responsive layout updates (global max-width changed from 820px to 1180px, component width constraints with clamp values), and router architecture changes (error boundary implementation, catch-all 404 route). While these changes appear intentional and related to the broader "self qa" quality assurance effort indicated by the PR title, they were not explicitly listed in issue #60's objectives and represent scope expansion beyond the stated requirements.
βœ… Passed checks (2 passed)
Check name Status Explanation
Linked Issues Check βœ… Passed The PR successfully addresses the primary coding objectives from issue #60. Footer height has been increased from 14.4rem to 18rem [Footer]; a completion message is now sent after solution flows with "μƒˆλ‘œμš΄ 문제λ₯Ό μ§ˆλ¬Έν•˜λ €λ©΄ 카메라λ₯Ό λˆŒλŸ¬μ£Όμ„Έμš”." [Solve]; and header back-navigation has been implemented with a new back button that conditionally displays on non-home pages [Header]. Token expiration handling has been enhanced with improved error routing logic in the interceptor [Interceptor], addressing that objective. All four main tasks from issue #60 are substantially implemented in this changeset.
Description Check βœ… Passed The PR description follows the required template structure and includes all necessary sections: the related issue (close #60) is clearly stated, the checklist is completed with all three items checked, and the tasks section provides context about verifying server functionality before merging. While the optional Screenshot and ETC sections are not provided, the required components are present and contain meaningful information that conveys the intent of the work.
✨ Finishing touches
  • πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/#60/self-qa

πŸ“œ Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between c3481b2 and 931f454.

πŸ“’ Files selected for processing (2)
  • src/routes/Layout.tsx (1 hunks)
  • src/shared/components/header/Header.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/shared/components/header/Header.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/Layout.tsx (1)
src/routes/routePath.ts (1)
  • routePath (1-11)
πŸ”‡ Additional comments (1)
src/routes/Layout.tsx (1)

9-13: LGTM! Clean addition to suppress header during OAuth callback.

Hiding the header on the Kakao OAuth callback page (/kakao/callback) is appropriate UXβ€”users shouldn't have navigation options during the authentication handshake. The implementation follows the existing pattern for LOGIN and SIGNUP paths.


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.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

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)

333-333: Disable toggle during upload to prevent race conditions.

The camera button remains enabled while isUploading is true, allowing users to initiate a new upload before the current one completes. This could lead to state conflicts and unexpected behavior.

Apply this fix:

-disabled={isPending}
+disabled={isPending || isUploading}
🧹 Nitpick comments (5)
src/pages/loginCallback/LoginCallback.tsx (1)

15-101: Consider adding cleanup for unmounted component scenarios.

While the ref flag prevents duplicate execution, adding a cleanup function would make this more robust against edge cases where the component unmounts during the async operation. This prevents calling navigate() after unmount.

Consider this pattern:

  useEffect(() => {
    if (hasRunRef.current) {
      return;
    }
    hasRunRef.current = true;
+   let isMounted = true;

    const params = new URLSearchParams(window.location.search);
    // ... existing validation code ...

    const handleLogin = async () => {
      try {
        // ... existing OAuth and API logic ...
-       navigate(routePath.SOLVE);
+       if (isMounted) {
+         navigate(routePath.SOLVE);
+       }
      } catch {
-       navigate(routePath.LOGIN);
+       if (isMounted) {
+         navigate(routePath.LOGIN);
+       }
      }
    };

    handleLogin();
+
+   return () => {
+     isMounted = false;
+   };
  }, [navigate]);
src/pages/solve/Solve.tsx (2)

210-211: LGTM: Upload flow refactor improves UX.

The immediate modal close (line 211) and loading indicator provide better user feedback. The sequential flow of upload β†’ preload β†’ display prevents layout shifts.

Consider these optional refinements:

  1. Add a brief delay before hiding the loading indicator (line 256):
 await preloadImages(uploadedUrls);
+const delay = new Promise(resolve => setTimeout(resolve, 100));
+await delay;

 uploadedUrls.forEach((url) => {
   handleImageSelect(url);
 });
 setIsUploading(false);
  1. Clarify variable naming (line 226):
-const uploadedUrls: string[] = [];
+const presignedDownloadUrls: string[] = [];

The current name uploadedUrls suggests the upload is complete, but it's populated during the upload loop.

Also applies to: 224-256


315-325: Consider adding accessibility attributes.

The loading indicator lacks semantic information for screen readers.

Add an aria-label for better accessibility:

 {isUploading && (
   <div className={styles.chatBubbleRight}>
-    <div className={styles.chatMyText}>
+    <div className={styles.chatMyText} role="status" aria-label="이미지 μ—…λ‘œλ“œ 쀑">
       <div className={styles.dots}>
         <span className={styles.dot} />
         <span className={styles.dot} />
         <span className={styles.dot} />
       </div>
     </div>
   </div>
 )}
src/pages/home/components/sectionBottom/SectionBottom.tsx (1)

23-23: Consider extracting magic number to named constant.

The change from +200 to +40 adjusts fade-in timing, which appears intentional given the layout height adjustments. However, the value 40 is a magic number.

Consider extracting to a named constant for clarity:

+ const FADE_IN_OFFSET = 40;
- const FADE_IN_START = offsetTop - window.innerHeight / 2 + 40;
+ const FADE_IN_START = offsetTop - window.innerHeight / 2 + FADE_IN_OFFSET;
src/pages/home/home.css.ts (1)

4-25: Responsive padding needs verification and adjustment for mobile screens.

The codebase does not implement responsive design patterns (no media queries or breakpoints found). The paddingRight: '2rem' (32px) on floatingSolveBtn is fixed across all screen sizes. Given the global minimum viewport width of 375px, this padding consumes approximately 8.5% of the screen width on mobile, which may cause layout issues. The maxWidth: '1180px' constraint is only effective on larger screens and doesn't address mobile spacing concerns.

Verify the button placement and padding on mobile viewports (particularly at 375px width) and consider:

  • Reducing padding for mobile devices
  • Adding responsive variants using vanilla-extract media queries if mobile support is needed
πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 6706475 and 66e4a6a.

πŸ“’ Files selected for processing (10)
  • src/pages/home/components/scrollText/ScrollText.tsx (1 hunks)
  • src/pages/home/components/sectionBottom/SectionBottom.tsx (2 hunks)
  • src/pages/home/components/sectionBottom/sectionBottom.css.ts (1 hunks)
  • src/pages/home/components/sectionTop/SectionTop.tsx (1 hunks)
  • src/pages/home/home.css.ts (1 hunks)
  • src/pages/loginCallback/LoginCallback.tsx (2 hunks)
  • src/pages/solve/Solve.tsx (6 hunks)
  • src/pages/solve/solve.css.ts (3 hunks)
  • src/shared/components/footer/footer.css.ts (1 hunks)
  • src/shared/styles/global.css.ts (1 hunks)
πŸ”‡ Additional comments (9)
src/pages/loginCallback/LoginCallback.tsx (1)

1-1: Prevents duplicate OAuth callback execution.

The useRef flag and guard clause correctly prevent the OAuth callback flow from executing multiple times, which is critical for avoiding duplicate token requests, signup/login calls, and navigation actions. This pattern is appropriate for one-time side effects like OAuth callbacks.

Also applies to: 13-13, 16-19

src/pages/solve/solve.css.ts (2)

62-66: LGTM: New chat text style follows existing patterns.

The chatMyText style appropriately uses the point color for user messages and follows the same structure as chatText and chatServerText.

Also applies to: 76-76


97-99: LGTM: Dot color override provides proper contrast.

The white dots on the point-colored background ensure the loading indicator remains visible in right-aligned chat bubbles.

src/pages/solve/Solve.tsx (2)

16-27: LGTM: Image preloading implementation is correct.

The concurrent preloading via Promise.all ensures all images are cached before display. Note that if any single image fails to load, the entire batch will rejectβ€”this is acceptable given the error handling in handleFileChange shows a generic retry message.


41-41: LGTM: Upload state properly integrated.

The isUploading state and its inclusion in the scroll effect dependency array ensure the loading indicator remains visible.

Also applies to: 53-53

src/shared/components/footer/footer.css.ts (1)

11-13: LGTM; confirm no overlap with fixed elements.

Height bump to 18rem looks fine. Just ensure pages with fixed-position elements (e.g., ScrollText/SectionTop) don’t overlap the footer at typical breakpoints.

src/pages/home/components/sectionBottom/sectionBottom.css.ts (1)

4-10: LGTM; verify reveal/timing still align after height increase.

Height -> 96rem reads fine. Please sanity-check SectionBottom reveal and ScrollText/SectionTop transitions so handoff points still feel synchronized.

src/pages/home/components/scrollText/ScrollText.tsx (1)

6-13: Thresholds misalignment confirmed after BREAKPOINT reduction.

Git history reveals the BREAKPOINT decreased from 2600 to 2380 (220px reduction), but the gradientOpacity window [2000, 2200] wasn't proportionally adjusted. This creates a 180px gap where the gradient stays fully opaque (value 1) while the element remains fixed, then switches to absolute positioning at 2380.

Originally: gradient ended at 2300, position switched at 2600 (~100px gap)
Currently: gradient ends at 2200, position switches at 2380 (180px gap)

Verify the visual sync across viewports to confirm this gap is intentional or requires realignment.

src/pages/home/components/sectionBottom/SectionBottom.tsx (1)

33-37: Verify the unusual translateYIn animation behavior is intentional.

The current animation maps output range [-110, -160], which moves the element upward (more negative y-value) as it fades in during scroll. This is atypicalβ€”most fade-in animations settle elements to their natural position (y: 0), not move them further away.

The element starts at top: 30% in the layout, and the animation adds these negative offsets, keeping it continuously offset upward throughout the fade-in. While this may be intentional parallax/stagger behavior coordinated with the 96rem section height, confirm:

  1. This upward movement during fade-in is the desired visual effect
  2. The -110 and -160 pixel offsets are correct values (currently undocumented magic numbers)
  3. No layout shift or cutoff issues occur with the section's overflow: hidden

@github-actions
Copy link

🎩 λΉŒλ“œμ— μ„±κ³΅ν–ˆμŠ΅λ‹ˆλ‹€!

Copy link

@coderabbitai coderabbitai bot left a 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

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)

217-279: Prevent concurrent uploads by disabling camera during upload.

There's no protection against the user clicking the camera button while isUploading is true. This could lead to concurrent uploads, state corruption, and confusing UI behavior.

The Toggle component should disable the camera button during uploads:

  <Toggle
    items={toggleItems}
    onTextSelect={handleTextSelect}
    onCameraClick={() => setIsOpen(true)}
-   disabled={isPending}
+   disabled={isPending || isUploading}
  />
🧹 Nitpick comments (4)
src/pages/home/components/sectionBottom/SectionBottom.tsx (1)

36-36: Verify the translateYIn direction and negative offset are correct.

The translateYIn mapping changed from [50, 0] to [-110, -160], which significantly alters the animation behavior:

  • Old: Element moves from 50px below its natural position to its natural position (0px) as you scroll.
  • New: Element starts 110px above its natural position and moves to 160px above (moving further upward).

This means the element will be permanently offset upward and will move even higher as the user scrolls. Please confirm this is the intended visual effect, especially in coordination with the increased wrapper height (94rem) mentioned in the AI summary.

Optionally, consider extracting these animation values to named constants for better maintainability:

+  const TRANSLATE_Y_START = -110;
+  const TRANSLATE_Y_END = -160;
+
   const translateYIn = useTransform(
     scrollY,
     [FADE_IN_START, FADE_IN_START + FADE_IN_GAP],
-    [-110, -160],
+    [TRANSLATE_Y_START, TRANSLATE_Y_END],
   );
src/shared/apis/interceptor.ts (1)

33-37: Prefer React Router navigation over full page reload.

Using window.location.href triggers a full page reload, which bypasses React Router, loses client-side state, and degrades UX. In a React Router app, programmatic navigation is preferred.

Consider these options:

  1. Import and call router.navigate('/error') if router instance is accessible
  2. Throw a specific error and handle navigation in a component-level error boundary
  3. Use a navigation service/hook pattern that the interceptor can invoke

Example using option 1 (if router is accessible):

+import { router } from '@/routes/router';

 // μ„œλ²„κ°€ κΊΌμ‘Œκ±°λ‚˜ λ„€νŠΈμ›Œν¬ 문제
 if (!error.response) {
-  window.location.href = '/error';
+  router.navigate('/error');
   return Promise.reject(error);
 }

Note: If the router instance isn't directly accessible in this module, consider implementing a navigation service pattern or handling this error type in a component-level boundary instead.

src/pages/solve/Solve.tsx (2)

16-27: Simplify the return statement.

The .then(() => undefined) on line 26 is unnecessary. Promise.all(promises) already returns Promise<void[]>, and TypeScript will accept this for a Promise<void> return type.

Apply this diff to simplify:

-  return Promise.all(promises).then(() => undefined);
+  return Promise.all(promises).then(() => {});

or even simpler:

-  return Promise.all(promises).then(() => undefined);
+  await Promise.all(promises);

and change the function signature to async:

-const preloadImages = (urls: string[]): Promise<void> => {
+const preloadImages = async (urls: string[]): Promise<void> => {

258-258: Consider partial preload recovery for better UX.

Currently, if any image fails to preload, all uploaded images are discarded and the user must retry the entire upload. Depending on your use case, you might want to show successfully preloaded images and only retry failed ones.

However, for math problem solving where both problem and solution images are expected, the current all-or-nothing behavior may be preferable to avoid showing incomplete information. If this is intentional, consider adding a comment explaining the decision.

Example of partial recovery (if desired):

const preloadImages = async (urls: string[]): Promise<string[]> => {
  const results = await Promise.allSettled(
    urls.map((url) => {
      return new Promise<string>((resolve, reject) => {
        const img = new Image();
        img.src = url;
        img.onload = () => resolve(url);
        img.onerror = () => reject(new Error(`Failed to load image: ${url}`));
      });
    })
  );
  
  return results
    .filter((r): r is PromiseFulfilledResult<string> => r.status === 'fulfilled')
    .map(r => r.value);
};

Also applies to: 269-276

πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 66e4a6a and c302132.

πŸ“’ Files selected for processing (8)
  • src/pages/home/components/scrollText/ScrollText.tsx (1 hunks)
  • src/pages/home/components/sectionBottom/SectionBottom.tsx (2 hunks)
  • src/pages/home/components/sectionBottom/sectionBottom.css.ts (1 hunks)
  • src/pages/home/components/sectionTop/SectionTop.tsx (2 hunks)
  • src/pages/home/components/sectionTop/sectionTop.css.ts (1 hunks)
  • src/pages/solve/Solve.tsx (7 hunks)
  • src/routes/router.tsx (1 hunks)
  • src/shared/apis/interceptor.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/pages/home/components/sectionTop/SectionTop.tsx
  • src/pages/home/components/scrollText/ScrollText.tsx
  • src/pages/home/components/sectionBottom/sectionBottom.css.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/router.tsx (2)
src/routes/routePath.ts (1)
  • routePath (1-11)
src/routes/globalRoutes.tsx (2)
  • publicRoutes (14-31)
  • protectedRoutes (33-54)
πŸ”‡ Additional comments (8)
src/pages/home/components/sectionTop/sectionTop.css.ts (1)

10-10: LGTM! Height adjustment aligns with layout refactor.

The height increase to a round 256rem is consistent with the broader home page layout updates mentioned in the AI summary (expanded container dimensions and padding in SectionTop.tsx).

Optionally verify that the fixed height works well across different viewport sizes, especially on smaller screens where 256rem might cause layout issues.

src/pages/home/components/sectionBottom/SectionBottom.tsx (1)

23-25: LGTM! Animation timing adjustments look intentional.

The changes to FADE_IN_START, FADE_IN_GAP, and FADE_OUT_GAP adjust the scroll animation timingβ€”starting the fade-in 200px earlier and shortening the transition duration. These appear to be deliberate polish adjustments coordinated with the layout changes mentioned in the AI summary.

src/shared/apis/interceptor.ts (1)

80-81: LGTM!

The comment clearly documents the fall-through behavior for other HTTP error codes.

src/routes/router.tsx (2)

13-17: LGTM! Error boundary provides crucial error resilience.

Wrapping GlobalLayout with an error boundary is excellent practiceβ€”it will catch React rendering errors and prevent the entire app from crashing.


26-29: Verify coordination with interceptor's /error redirect.

The catch-all * route will handle 404s and also catch the /error redirect from interceptor.ts (lines 35). Since /error is not explicitly defined in routePath.ts, it will match this wildcard route and render the <Error /> component.

Please confirm this is the intended behaviorβ€”that network/server errors from the interceptor should display the same <Error /> component as 404s.

If you want different UIs for network errors vs. 404s, consider:

  1. Adding an explicit /error route before the catch-all
  2. Passing error context (e.g., via URL params or router state) to differentiate error types

Example of adding an explicit error route:

   },
+  {
+    path: '/error',
+    element: <Error type="network" />,
+  },
   {
     path: '*',
-    element: <Error />,
+    element: <Error type="404" />,
   },
 ]);
src/pages/solve/Solve.tsx (3)

42-42: LGTM! Uploading state properly integrated.

The isUploading state is correctly initialized and added to the scroll effect dependencies, ensuring the chat scrolls appropriately during the upload process.

Also applies to: 54-54


323-333: LGTM! Uploading indicator provides clear feedback.

The uploading indicator correctly mirrors the server-side loading pattern and provides clear visual feedback to users during the upload process. The placement on the right side (user's side) is appropriate.


228-228: LGTM! Input reset properly handled in all paths.

The input value is now correctly reset in both the early return path (line 228) and the finally block (line 278), ensuring the file input is always cleared regardless of success or failure. This prevents issues with re-selecting the same file.

Also applies to: 277-279

@github-actions
Copy link

🎩 λΉŒλ“œμ— μ„±κ³΅ν–ˆμŠ΅λ‹ˆλ‹€!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/pages/home/components/sectionTop/SectionTop.tsx (1)

82-87: Consider using the CSS variable for consistency.

While maxWidth: 1180 is valid (React treats numeric values as pixels), using var(--max-width) would be more maintainable since the global max-width was updated to 1180px in the root CSS variables. This eliminates the magic number and ensures consistency if the global value changes.

Apply this diff to use the CSS variable:

-            maxWidth: 1180,
+            maxWidth: 'var(--max-width)',
πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between c302132 and e174401.

πŸ“’ Files selected for processing (3)
  • src/pages/home/components/sectionBottom/SectionBottom.tsx (2 hunks)
  • src/pages/home/components/sectionBottom/sectionBottom.css.ts (1 hunks)
  • src/pages/home/components/sectionTop/SectionTop.tsx (2 hunks)
πŸ”‡ Additional comments (5)
src/pages/home/components/sectionTop/SectionTop.tsx (2)

89-95: LGTM! Responsive sizing implemented well.

The responsive width using clamp(425.5px, 60vw, 600px) with height: 'auto' is a solid approach that maintains the aspect ratio across different viewport sizes. The willChange: 'opacity' hint appropriately optimizes the animated opacity transform.


108-114: LGTM! Consistent performance optimization.

Adding the explicit willChange: 'opacity' style object to IcMainChat1 brings it in line with the other chat components (Chat2 and Chat3), ensuring consistent performance hints for all animated elements.

src/pages/home/components/sectionBottom/sectionBottom.css.ts (1)

7-7: LGTM! Layout height adjustment aligns with broader home page refinements.

The height increase from 88.8rem to 93rem is a straightforward layout adjustment that coordinates with related changes in other home sections mentioned in the AI summary.

src/pages/home/components/sectionBottom/SectionBottom.tsx (2)

23-25: Verify animation timing across different viewport sizes.

The animation now starts later (removed +200 offset from FADE_IN_START) and completes faster (gaps reduced from 200 to 150). These changes affect when and how quickly the section fades in during scroll.

Please test this across different screen sizes to ensure the section animates smoothly without appearing too late or too abruptly.


33-37: Verify the translateYIn range is intentional.

The transform range changed from [50, 0] to [-100, -150], which means:

  • Previous: Element animated from 50px below to its natural position (0)
  • Current: Element stays at -100px above and moves to -150px as you scroll (never reaching its natural position)

This is an unusual patternβ€”the element moves further upward during scroll and never settles at y: 0. While this could be an intentional parallax effect working with the top: '30%' CSS positioning, please verify this produces the desired visual behavior.

@github-actions
Copy link

🎩 λΉŒλ“œμ— μ„±κ³΅ν–ˆμŠ΅λ‹ˆλ‹€!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/pages/solve/Solve.tsx (1)

229-246: Critical: Loading state starts before validation, causing UX issues.

There are two related problems:

  1. Modal closes before validation (line 231): The modal closes immediately on file selection, but if validation fails (lines 234-241), the user sees the modal disappear and then an error appearsβ€”creating a jarring, confusing experience.

  2. Loading state not cleaned up on validation failure (lines 244-246): The uploading state (isUploading, uploadingSlots) is initialized before validation. If validation fails at lines 234-241, the early return at line 241 leaves the loading indicators visible indefinitely.

Apply this diff to fix both issues:

  const handleFileChange = async (e: React.ChangeEvent<HTMLInputElement>) => {
-    // 1. 파일 선택 μ¦‰μ‹œ λͺ¨λ‹¬ λ‹«κΈ°
-    setIsOpen(false);
-
    const files = e.target.files;
    if (!files || files.length < expectedCount) {
      addServerMessage(
        expectedCount === 1
          ? '문제 이미지 1μž₯을 μ„ νƒν•΄μ£Όμ„Έμš”.'
          : '문제 이미지 1μž₯, 풀이 이미지 1μž₯을 μ„ νƒν•΄μ£Όμ„Έμš”.',
      );
      e.target.value = '';
      return;
    }

-    // 2. 'me' λ‘œλ”© UI μ‹œμž‘ 및 μ΄ˆκΈ°ν™”
+    // 1. 파일 검증 ν›„ λͺ¨λ‹¬ λ‹«κΈ°
+    setIsOpen(false);
+    
+    // 2. 'me' λ‘œλ”© UI μ‹œμž‘ 및 μ΄ˆκΈ°ν™”
    setUploadingSlots(Array.from({ length: expectedCount }, (_, i) => i));
    setIsUploading(true);
    const uploadedUrls: string[] = [];
πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between e174401 and c3481b2.

β›” Files ignored due to path filters (1)
  • public/svg/ic_left_arrow.svg is excluded by !**/*.svg
πŸ“’ Files selected for processing (6)
  • src/pages/my/my.css.ts (1 hunks)
  • src/pages/solve/Solve.tsx (9 hunks)
  • src/shared/components/header/Header.tsx (2 hunks)
  • src/shared/components/header/header.css.ts (1 hunks)
  • src/shared/components/icons/IcLeftArrow.tsx (1 hunks)
  • src/shared/components/icons/index.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/shared/components/header/Header.tsx (1)
src/routes/routePath.ts (1)
  • routePath (1-11)
src/pages/solve/Solve.tsx (3)
src/pages/solve/ChatManager.tsx (1)
  • Chat (3-7)
src/pages/solve/apis/axios.ts (1)
  • getPresignedUrl (6-15)
src/shared/apis/upload.ts (1)
  • uploadToPresignedUrl (1-15)
πŸ”‡ Additional comments (9)
src/pages/my/my.css.ts (1)

149-150: LGTM! Layout constraints align with refactor goals.

The addition of width: '100%' and maxWidth: '60rem' to the chip list wrapper is consistent with the broader layout tightening in this PR. The 960px max width provides appropriate constraints for the chip list while allowing full expansion on smaller screens.

src/shared/components/icons/index.ts (1)

16-16: LGTM!

The export is correctly placed in alphabetical order and follows the established pattern.

src/shared/components/header/header.css.ts (1)

17-26: LGTM!

The new layout groups provide clean separation for left and right header sections with appropriate flex styling.

src/shared/components/icons/IcLeftArrow.tsx (1)

1-18: LGTM!

The icon component follows the standard SVG pattern with proper prop spreading, allowing callers to override attributes including color.

src/shared/components/header/Header.tsx (1)

26-69: LGTM! Clean refactor with proper conditional rendering.

The header now correctly:

  • Shows the back button on appropriate pages (excluding home, solve, and my page)
  • Conditionally renders the MyPage button when not on the my page
  • Uses consistent icon colors derived from the isHome prop
  • Includes proper accessibility labels
src/pages/solve/Solve.tsx (4)

16-27: LGTM! Clean image preloading implementation.

The concurrent preloading approach ensures images are cached before display, preventing visual flicker in the chat interface.


206-211: βœ… Completion message now sent as intended.

This addresses the PR objective: "Send a completion message even when viewing all solutions." The delayed message properly guides users to start a new problem.


272-282: LGTM! Preload-then-display pattern prevents visual glitches.

The flow correctly:

  1. Uploads to S3
  2. Preloads images into browser cache
  3. Clears loading indicators
  4. Displays images (which now render instantly from cache)

This ensures smooth UX without image pop-in.


343-354: LGTM! Loading indicators properly reflect upload progress.

The uploading slots map ensures each image gets its own loading bubble, providing clear visual feedback during multi-file uploads.

@github-actions
Copy link

🎩 λΉŒλ“œμ— μ„±κ³΅ν–ˆμŠ΅λ‹ˆλ‹€!

@hansoojeongsj hansoojeongsj merged commit 2d86688 into develop Oct 28, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor] λ‚˜ν™€λ‘œ QA

2 participants