fix: prevent background scroll when modal is open

[qian403]

• Add overlay backdrop to team member modal
• Add scroll lock to transport dialog
• Use position: fixed method for iOS Safari compatibility

Fixes #127

[Copilot]

Pull request overview

This PR fixes a bug where background content could still scroll and be interacted with when modals are open. The fix implements a scroll lock mechanism using the position: fixed approach for better iOS Safari compatibility.

Changes:

• Added scroll lock functionality to the transport dialog using position: fixed with scroll position preservation
• Added overlay backdrop to the member info modal with click-to-close functionality
• Modified mobile modal animation to slide from bottom instead of left

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 15 comments.

File	Description
src/pages/info/index.astro	Added scroll lock/unlock logic to transport dialog with position: fixed approach for iOS compatibility
src/components/page/MemberInfo.astro	Added overlay backdrop element, scroll lock logic for modal, and updated mobile layout positioning

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When the modal opens, focus should be moved to the modal or its close button for better keyboard navigation and accessibility. Currently, focus remains on the trigger element. Consider adding modal.focus() or closeBtn.focus() after showing the modal to ensure keyboard users can immediately interact with or close the modal without having to tab through background elements.

Suggested change

	modal.classList.add("show");
	overlay?.classList.add("show");
	// Move focus into the modal (to the close button) for accessibility
	const closeBtn = modal.querySelector(".close-btn") as HTMLButtonElement | null;
	modal.classList.add("show");
	overlay?.classList.add("show");
	// Ensure keyboard users can immediately interact with or close the modal
	if (closeBtn) {
	closeBtn.focus();
	}

[Copilot]

The scroll lock/unlock logic is duplicated between this file and src/pages/info/index.astro (lines 196-205). Consider extracting this into a shared utility function to reduce duplication and ensure consistency. This would make future maintenance easier and reduce the risk of the implementations diverging.

[Copilot]

The overlay z-index is set to 100, which conflicts with the Nav component (src/components/Nav.astro:151) that also uses z-index: 100. This could cause layering issues where the navigation might appear above the modal overlay. Consider increasing the overlay z-index to at least 102 (modal is 101) to ensure proper stacking order, or better yet, use a higher range like 1000+ for modal overlays to avoid conflicts with other fixed elements.

[Copilot]

In the mobile breakpoint, the modal animation direction has changed from translateX(-105%) (slide from left) to translateY(100%) (slide from bottom). This changes the user experience on mobile. If this is intentional to improve UX on mobile devices, this is good. However, if this was an unintentional change while adding the position: fixed scroll lock, it should be reviewed to maintain consistency with the original design intent.

Suggested change

	transform: translateY(100%);
	}
	.member-info.show {
	transform: translateY(0);
	transform: translateX(-105%);
	}
	.member-info.show {
	transform: translateX(0);

[Copilot]

The overlay element should have appropriate ARIA attributes for accessibility. Consider adding role="presentation" or aria-hidden="true" when hidden, and removing it when shown. This helps screen readers understand that the overlay is a visual element and not interactive content. Additionally, the overlay should prevent focus from moving to elements behind it when the modal is open.

Suggested change

	<div id={`${id}-overlay`} class="member-info-overlay"></div>
	<div id={id} class="member-info">
	<div
	id={`${id}-overlay`}
	class="member-info-overlay"
	role="presentation"
	aria-hidden="true"
	></div>
	<div
	id={id}
	class="member-info"
	role="dialog"
	aria-modal="true"
	aria-labelledby="member-name"
	aria-describedby="member-description"
	>

[Copilot]

The unlockBodyScroll function doesn't check if a scroll lock is currently active before attempting to unlock. If called when no modal is open (e.g., double-clicking close button or pressing ESC multiple times), it could attempt to restore a scroll position that doesn't exist or is from a previous session. Consider adding a guard check to verify that dataset.scrollY exists before attempting restoration, or using a flag to track lock state.

[Copilot]

Setting document.body.style.width = "100%" when locking scroll may cause layout issues on pages where the body has a different width configuration or uses max-width. On pages with scrollbars, this could cause a layout shift when the scrollbar disappears (as the body becomes fixed). Consider also setting the right padding to match the scrollbar width to prevent content shift: document.body.style.paddingRight = ${window.innerWidth - document.documentElement.clientWidth}px

Suggested change

	document.body.style.width = "100%";
	const scrollBarWidth = window.innerWidth - document.documentElement.clientWidth;
	if (scrollBarWidth > 0) {
	document.body.style.paddingRight = `${scrollBarWidth}px`;
	}

[Copilot]

The removed CSS properties 'align-self: center' and 'justify-self: center' were likely used for centering the modal. With the new approach using 'top: 2rem' and 'left: 2rem', the modal is now positioned explicitly. Ensure this change doesn't break the layout on various mobile screen sizes, particularly on very small or very large mobile devices.

[Copilot]

Setting document.body.style.width = "100%" when locking scroll may cause layout issues on pages where the body has a different width configuration. On pages with scrollbars, this could cause a layout shift when the scrollbar disappears. Consider also setting padding-right to compensate for the scrollbar width to prevent content shift.

[Copilot]

The unlockBodyScroll function uses delete document.body.dataset.scrollY to clean up, but it doesn't verify that all style properties are fully cleared. If the unlock function is called when styles weren't set (edge case), setting them to empty strings is harmless. However, consider also checking if dataset.scrollY exists before attempting deletion to make the function more defensive.

Suggested change

	const scrollY = document.body.dataset.scrollY || "0";
	document.body.style.overflow = "";
	document.body.style.position = "";
	document.body.style.width = "";
	document.body.style.top = "";
	window.scrollTo(0, parseInt(scrollY, 10));
	delete document.body.dataset.scrollY;
	const storedScrollY = document.body.dataset.scrollY;
	document.body.style.overflow = "";
	document.body.style.position = "";
	document.body.style.width = "";
	document.body.style.top = "";
	const scrollY = typeof storedScrollY === "string" ? parseInt(storedScrollY, 10) || 0 : window.scrollY || 0;
	window.scrollTo(0, scrollY);
	if ("scrollY" in document.body.dataset) {
	delete document.body.dataset.scrollY;
	}