fix(Modal, AddContactModal): Fix Storybook docs mode rendering issues#40
Conversation
Summary: Fixes multiple issues with Modal components in Storybook docs mode where multiple stories are rendered inline on the same page. Modal.tsx: - Add isStorybookDocsMode() helper to detect Storybook docs view - Skip scroll lock (body overflow: hidden) in docs mode to allow page scroll - Add multi-modal support with openModalCount reference counter - Preserve and restore original body overflow correctly useFocusTrap.ts: - Add isStorybookDocsMode() helper to detect Storybook docs view - Skip focus trap in docs mode to prevent auto-scroll to middle of page - Focus trap still works normally in canvas mode and production AddContactModal.stories.tsx: - Rewrite stories to use InteractiveWrapper that syncs with Storybook args - Add proper argTypes for all props including contact, onOpenChange, onSave - Add default args at meta level for consistent behavior - Change layout to fullscreen with iframeHeight: 700 for better visibility - All stories now render with working controls in docs page - Remove redundant wrapper components Issues Fixed: 1. No controls displayed on AddContactModal docs page 2. Page scroll broken in docs mode (body overflow locked) 3. Page auto-scrolled to middle on load (focus trap focusing elements) 4. Modal examples cut off in docs page (insufficient container height)
There was a problem hiding this comment.
Pull request overview
This PR fixes Storybook docs mode rendering issues for Modal components where multiple stories are rendered inline on the same page. The changes prevent body scroll locking and focus trapping in docs mode, which were causing page scroll issues and auto-scrolling behavior.
Changes:
- Added
isStorybookDocsMode()detection to skip scroll lock and focus trap in Storybook docs view - Implemented multi-modal support with reference counter for proper body overflow management
- Refactored AddContactModal stories to use InteractiveWrapper with proper args synchronization and improved layout
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/hooks/useFocusTrap.ts | Added Storybook docs mode detection to skip focus trap and prevent auto-scroll in docs view |
| src/components/Modal/Modal.tsx | Added docs mode detection, multi-modal counting, and body overflow management to prevent scroll issues |
| src/components/AddContactModal/AddContactModal.stories.tsx | Refactored stories to use InteractiveWrapper with proper args, removed redundant wrappers, changed to fullscreen layout |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function isStorybookDocsMode(): boolean { | ||
| if (typeof window === 'undefined') return false; | ||
| return window.location.search.includes('viewMode=docs'); | ||
| } |
There was a problem hiding this comment.
The isStorybookDocsMode() function is duplicated in both useFocusTrap.ts and Modal.tsx. This creates a maintainability issue - any changes to the detection logic need to be applied in multiple places. Consider extracting this function to a shared utility file (e.g., src/utils/storybook.ts or src/utils/environment.ts) and importing it where needed.
| // Track number of open modals to manage body scroll lock | ||
| let openModalCount = 0; | ||
| let originalBodyOverflow: string | null = null; |
There was a problem hiding this comment.
The module-level variables openModalCount and originalBodyOverflow can cause issues in testing environments, hot module replacement (HMR), and server-side rendering scenarios. These values persist across component unmounts and remounts, which could lead to incorrect state if components are rapidly mounted/unmounted or if HMR reloads occur. Consider using a ref-based approach or React context to manage this state more safely, or at minimum add a reset mechanism for testing environments.
| React.useEffect(() => { | ||
| if (open) { | ||
| const originalOverflow = document.body.style.overflow; | ||
| document.body.style.overflow = 'hidden'; | ||
| if (open && !isStorybookDocsMode()) { | ||
| openModalCount++; | ||
| // Only capture and set overflow when first modal opens | ||
| if (openModalCount === 1) { | ||
| originalBodyOverflow = document.body.style.overflow; | ||
| document.body.style.overflow = 'hidden'; | ||
| } | ||
| return () => { | ||
| document.body.style.overflow = originalOverflow; | ||
| openModalCount--; | ||
| // Only restore overflow when last modal closes | ||
| if (openModalCount === 0 && originalBodyOverflow !== null) { | ||
| document.body.style.overflow = originalBodyOverflow; | ||
| originalBodyOverflow = null; | ||
| } | ||
| }; | ||
| } | ||
| }, [open]); |
There was a problem hiding this comment.
When isStorybookDocsMode() returns true, the useEffect doesn't return a cleanup function. This means if the modal is open and then the open prop changes to false while still in Storybook docs mode, the effect cleanup won't decrement the openModalCount. This could lead to a memory leak or incorrect counter state. The cleanup function should always be returned, even when in Storybook docs mode, to ensure proper cleanup of the effect subscription.
1. Extract isStorybookDocsMode() to shared utility (src/utils/environment.ts) - Removes duplication between Modal.tsx and useFocusTrap.ts - Single source of truth for Storybook docs mode detection 2. Replace module-level variables with scrollLockState object - Safer for HMR, testing, and SSR scenarios - Includes reset() method for testing environments - Exports __resetScrollLockState for test cleanup 3. Fix useEffect cleanup function - Always return cleanup function (or undefined) explicitly - Prevents potential memory leaks when open prop changes in docs mode
Deploying ui with
|
| Latest commit: |
a480e38
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://94fe2db5.ui-6d0.pages.dev |
| Branch Preview URL: | https://feature-add-contact-modal-up.ui-6d0.pages.dev |
Summary: Fixes multiple issues with Modal components in Storybook docs mode where multiple stories are rendered inline on the same page.
Modal.tsx:
useFocusTrap.ts:
AddContactModal.stories.tsx:
Issues Fixed:
add-contact-modal.mov