Skip to content

Conversation

@Recxsmacx
Copy link

@Recxsmacx Recxsmacx commented Dec 5, 2025

fixes #4674
image

Summary by CodeRabbit

  • Improvements
    • Flyout menu now automatically enables scrolling when content exceeds the available viewport height, improving overflow handling and usability.
  • Chores
    • Internal styling normalized for consistent rendering; no public API or props were changed.

✏️ Tip: You can customize this high-level summary in your review settings.

@netlify
Copy link

netlify bot commented Dec 5, 2025

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 269a353
🔍 Latest deploy log https://app.netlify.com/projects/asyncapi-website/deploys/6932efbbe9fa1b00080b25c9
😎 Deploy Preview https://deploy-preview-4675--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

Walkthrough

Adds internal scrolling to FlyoutMenu: measures content height vs 80% of viewport, sets a needsScroll flag, and conditionally applies max-height and overflow-y to enable internal scrolling when content exceeds the threshold.

Changes

Cohort / File(s) Summary
Internal scrolling & rendering update
components/navigation/FlyoutMenu.tsx
Adds contentRef: HTMLDivElement, needsScroll state and a useEffect to compare content height against 80% of viewport; conditionally applies max-h-[80vh] and overflow-y-auto to the inner container; switches class-string quoting to double quotes; updates data-testid quoting.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus areas:
    • Verify the 80% viewport height calculation and handling of padding/margins.
    • Confirm the useEffect dependency array and cleanup (if needed) to avoid extra reflows.
    • Ensure scroll behavior and applied classes don't conflict with existing keyboard/focus interactions.

Suggested labels

ready-to-merge

Suggested reviewers

  • derberg
  • akshatnema
  • devilkiller-ag
  • anshgoyalevil
  • sambhavgupta0705

Poem

🐰 I hopped in swift to mend the view,

When menus grew, I knew what to do.
Eighty percent—then scroll with ease,
Gentle bounce and quiet breeze.
Hooray! The flyout fits with glee. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix: nav flyout menu issue' is vague and generic, using non-descriptive language that doesn't clearly convey what specific problem is being fixed. Replace with a more specific title that describes the actual fix, such as 'fix: add scrolling to nav flyout menu when content exceeds viewport' or similar.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR addresses the nav flyout menu issue by implementing internal scrolling logic to prevent screen breaking when nav items exceed viewport height.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the flyout menu scrolling behavior; refactoring from single to double quotes is a minor incidental change without functional impact.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6900135 and de41b71.

📒 Files selected for processing (1)
  • components/navigation/FlyoutMenu.tsx (2 hunks)
🔇 Additional comments (3)
components/navigation/FlyoutMenu.tsx (3)

1-1: LGTM!

The added React hooks are necessary for the scroll detection logic and are all properly utilized in the component.


15-16: LGTM!

The ref and state initialization are properly typed and appropriately set to initial values.


34-37: Good fix: calculation now matches the applied CSS.

The threshold calculation (80vh at line 21) now correctly matches the applied CSS class (max-h-[80vh] at line 36), resolving the inconsistency flagged in the previous review. The contentRef is properly attached and the conditional styling logic is correct.


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.

@Recxsmacx Recxsmacx changed the title fixed nav flyout menu issue fix : Nav flyout menu issue Dec 5, 2025
@Recxsmacx Recxsmacx changed the title fix : Nav flyout menu issue fix: nav flyout menu issue Dec 5, 2025
@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Dec 5, 2025

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 44
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

Lighthouse ran on https://deploy-preview-4675--asyncapi-website.netlify.app/

Copy link
Contributor

@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

🧹 Nitpick comments (1)
components/navigation/FlyoutMenu.tsx (1)

33-38: Good approach with conditional scroll styling.

The ref placement and conditional application of max-h-[70vh] overflow-y-auto correctly implements the scroll behavior to prevent the flyout from breaking the screen layout. This addresses the reported bug effectively.

Note on quote style changes (lines 28-37): The changes from single quotes to double quotes are purely stylistic and not necessary for fixing the bug. While consistency is good, these changes add noise to the diff.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3fc64a4 and 6900135.

📒 Files selected for processing (1)
  • components/navigation/FlyoutMenu.tsx (2 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Redirect rules - asyncapi-website
  • GitHub Check: Header rules - asyncapi-website
  • GitHub Check: Pages changed - asyncapi-website
  • GitHub Check: Lighthouse CI
🔇 Additional comments (2)
components/navigation/FlyoutMenu.tsx (2)

1-1: LGTM!

The additional React hooks are correctly imported and necessary for the scroll detection logic.


15-16: LGTM!

The ref and state declarations are correctly typed and initialized.

@princerajpoot20 princerajpoot20 self-requested a review as a code owner December 5, 2025 14:44
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.

[BUG] Nav items toggle screen vreaking

3 participants