Skip to content

Conversation

@hwangdae
Copy link
Collaborator

@hwangdae hwangdae commented Sep 12, 2025

작업 내용

문제점 및 어려움

해결 방안

공유 사항

Summary by CodeRabbit

  • New Features

    • None.
  • Refactor

    • Simplified page transition behavior: forward navigations animate as push, back navigations animate as pop, and replace navigations show no animation for a cleaner, more consistent experience.
  • Style

    • Adjusted navigation animations to feel more natural across route changes.
  • Bug Fixes

    • Removed automatic scroll-to-top on navigation; pages now retain their scroll position when moving between routes.

@coderabbitai
Copy link

coderabbitai bot commented Sep 12, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Removes PageTransitionWrapper usage from Router and refactors its internal navigation/animation logic. Eliminates scroll-to-top side effect, adjusts how push/pop/replace animations are determined based on isBack and navigateType, and renders Routes directly in Router without the wrapper.

Changes

Cohort / File(s) Summary
Page transition logic refactor
apps/nowait-user/src/components/layout/PageTransitionWrapper.tsx
Removed useEffect scroll-to-top; simplified animation selection: push → "navigate-push", back (isBack) → "navigate-pop", replace (non-back) → ""; removed isNavigatePop; constrained replace/push to !isBack. Minor formatting.
Router unwrapping
apps/nowait-user/src/routes/Router.tsx
Removed PageTransitionWrapper import and wrapper; Router now renders <Routes location={location}> directly. No route definition changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant U as User
    participant R as Router
    participant PTW as PageTransitionWrapper
    participant RT as Routes

    rect rgba(230, 240, 255, 0.5)
    Note over R,PTW: Previous flow
    U->>R: Navigate
    R->>PTW: Render with location
    PTW->>PTW: Decide animation<br/>(isBack / navigateType)
    PTW->>RT: Render routes with transition classes
    end
Loading
sequenceDiagram
    autonumber
    participant U as User
    participant R as Router
    participant RT as Routes

    rect rgba(235, 255, 235, 0.5)
    Note over R,RT: New flow
    U->>R: Navigate
    R->>RT: Render routes directly
    end

    Note over U,R: Animation decision now internal to PTW (refactored), but wrapper not used by Router.
Loading
sequenceDiagram
    autonumber
    participant Nav as Navigation State
    participant Anim as Animation Selector

    rect rgba(255, 248, 225, 0.6)
    Note over Nav,Anim: Refined animation logic
    Nav->>Anim: isBack, navigateType
    alt isBack == true
        Anim-->>Nav: "navigate-pop"
    else navigateType == "PUSH" && !isBack
        Anim-->>Nav: "navigate-push"
    else navigateType == "REPLACE" && !isBack
        Anim-->>Nav: "" (no animation)
    end
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • oriNuguri25
  • dgKim1

Poem

I hop through routes without a fuss,
No wrapper rides the render bus.
Push and pop, I time my leap—
Replace stays quiet, calm and deep.
Scroll-to-top? I let it be.
Smooth as clover, shipping free! 🐇✨

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


📜 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 53bb1e8 and 6504971.

📒 Files selected for processing (2)
  • apps/nowait-user/src/components/layout/PageTransitionWrapper.tsx (2 hunks)
  • apps/nowait-user/src/routes/Router.tsx (0 hunks)
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch develop

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.

@hwangdae hwangdae merged commit 20a1837 into main Sep 12, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants