Skip to content

fix: prevent background scroll when roadmap modal is open #4710

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.

Sign up for GitHub

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

Open
sammy200-ui wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: prevent background scroll when roadmap modal is open #4710

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion components/Modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,17 @@ interface IModalProps {
export default function Modal({ title, children, onModalClose = () => {} }: IModalProps) {
const modalRef = useRef<HTMLDivElement>(null);

// Focus the modal when it mounts
// Focus the modal when it mounts and lock body scroll
useEffect(() => {
if (modalRef.current) modalRef.current.focus();

// Prevent background scroll when modal is open
document.body.style.overflow = 'hidden';

// Cleanup: restore scroll when modal is closed
return () => {
document.body.style.overflow = '';
Comment on lines +23 to +27
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The current implementation doesn't handle the case where multiple Modal instances might be mounted simultaneously. If two modals are open and the first one closes, it will restore body scroll even though the second modal is still open. Consider implementing a reference counting mechanism or using a shared state manager to track the number of open modals, and only restore overflow when the count reaches zero.

Suggested change
document.body.style.overflow = 'hidden';
// Cleanup: restore scroll when modal is closed
return () => {
document.body.style.overflow = '';
openModalCount += 1;
if (openModalCount === 1) {
document.body.style.overflow = 'hidden';
}
// Cleanup: restore scroll when modal is closed
return () => {
openModalCount -= 1;
if (openModalCount === 0) {
document.body.style.overflow = '';
}

Copilot uses AI. Check for mistakes.
};
}, []);
Comment on lines +18 to 29
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 14, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, examine the Modal component file structure
find . -type f -name "Modal.tsx" | head -5

Repository: asyncapi/website

Length of output: 82

🏁 Script executed:

# Check for all references to body.style.overflow in the codebase
rg "body\.style\.overflow|document\.body\.style\.overflow" --type tsx --type ts -n

Repository: asyncapi/website

Length of output: 87

🏁 Script executed:

# Search for Modal usage patterns to understand stacking scenarios
rg "Modal|<Modal" --type tsx --type ts -B 2 -A 2 | head -100

Repository: asyncapi/website

Length of output: 87

🏁 Script executed:

# Check for overscroll-behavior in CSS or Tailwind configs
rg "overscroll|scroll-contain" --type css --type tsx -n

Repository: asyncapi/website

Length of output: 87

🏁 Script executed:

# Look at the complete Modal component to understand context
cat -n components/Modal.tsx 2>/dev/null || find . -path "*/components/Modal.tsx" -exec cat -n {} \;

Repository: asyncapi/website

Length of output: 3256

🏁 Script executed:

# Search for Modal imports and usage
grep -r "Modal" --include="*.tsx" --include="*.ts" --include="*.jsx" --include="*.js" | grep -i "import\|<Modal" | head -20

Repository: asyncapi/website

Length of output: 326

🏁 Script executed:

# Search for body.style.overflow without file type restriction
grep -r "body\.style\.overflow\|document\.body\.style\.overflow" . 2>/dev/null | grep -v node_modules

Repository: asyncapi/website

Length of output: 191

🏁 Script executed:

# Check if there are multiple Modal instances rendered simultaneously anywhere
grep -r "<Modal" --include="*.tsx" --include="*.jsx" -B 5 -A 5 | head -60

Repository: asyncapi/website

Length of output: 721

🏁 Script executed:

# Look for any overflow manipulation in CSS or global styles
find . -name "*.css" -o -name "tailwind.config.*" | xargs grep -l "overflow\|overscroll" 2>/dev/null | head -10

Repository: asyncapi/website

Length of output: 101

🏁 Script executed:

# Check globals.css for any overflow or body styles
cat -n ./styles/globals.css | head -100

Repository: asyncapi/website

Length of output: 2501

🏁 Script executed:

# Check tailwind.config.ts for overscroll or scroll-related settings
cat -n ./tailwind.config.ts

Repository: asyncapi/website

Length of output: 7484

Restore previous body.style.overflow as defensive programming for potential modal stacking.

The current cleanup sets document.body.style.overflow = '', which could cause issues if (a) the modal component is reused in contexts where multiple modals stack, or (b) body.style.overflow is set elsewhere in the future. While the current codebase only renders a single Modal instance at a time (in RoadmapPill.tsx), storing and restoring the previous value follows best practices and prevents regressions:

 export default function Modal({ title, children, onModalClose = () => {} }: IModalProps) {
   const modalRef = useRef<HTMLDivElement>(null);
+  const prevBodyOverflowRef = useRef<string>('');
 
   // Focus the modal when it mounts and lock body scroll
   useEffect(() => {
     if (modalRef.current) modalRef.current.focus();
 
     // Prevent background scroll when modal is open
-    document.body.style.overflow = 'hidden';
+    prevBodyOverflowRef.current = document.body.style.overflow;
+    document.body.style.overflow = 'hidden';
 
     // Cleanup: restore scroll when modal is closed
     return () => {
-      document.body.style.overflow = '';
+      document.body.style.overflow = prevBodyOverflowRef.current;
     };
   }, []);

The optional overscroll-behavior: contain mitigation is not currently needed, as the modal uses fixed positioning and doesn't exhibit scroll chaining behavior.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In components/Modal.tsx around lines 18–29, the effect currently sets
document.body.style.overflow = 'hidden' and restores it to '' on cleanup; change
it to capture the previous document.body.style.overflow value before
overwriting, set overflow to 'hidden' on mount, and restore the saved previous
value on cleanup so stacked modals or external overflow modifications are
preserved (save value to a local variable outside the cleanup and assign it back
in the return function).


/**
Expand Down