fix: resolve modal stacking, animation, and close cleanup bugs

- Set backdrop z-index:0 and content z-index:1 to fix backdrop covering
  modal content within the stacking context
- Force reflow between removing hidden and adding open class so CSS
  opacity transition fires correctly
- Add transitionend listener + setTimeout fallback to restore hidden
  class after close animation completes
- Override flex:1 on modal footer buttons to prevent full-width stretch
- Add min-width to modal size variants for consistent sizing
- Add 23 tests covering open/close lifecycle, keyboard, and destroy

Amit Haridas

src/styles/modal.css

@@ -14,7 +14,7 @@
     background: rgba(0, 0, 0, 0.4);
     backdrop-filter: blur(4px);
     -webkit-backdrop-filter: blur(4px);
-    z-index: var(--z-modal, 200);
+    z-index: 0;
     cursor: pointer;
 }
 
@@ -46,6 +46,9 @@
  * ============================================ */
 
 .modal-content {
+    position: relative;
+    z-index: 1;
+    width: 100%;
     background: hsl(var(--background, 0 0% 100%));
     border-radius: var(--radius-lg, 0.5rem);
     box-shadow: var(--shadow-xl, 0 20px 25px -5px rgb(0 0 0 / 0.1), 0 8px 10px -6px rgb(0 0 0 / 0.1));
@@ -139,6 +142,9 @@
 
 .modal-footer .btn {
     min-width: 80px;
+    /* styles-modern.css sets flex:1 on .btn-primary/.btn-secondary globally
+     * which causes footer buttons to stretch to full width. Override here. */
+    flex: none;
 }
 
 /* ============================================
@@ -208,15 +214,18 @@
 
 .modal-content.small {
     max-width: 400px;
+    min-width: 320px;
 }
 
 .modal-content.large {
     max-width: 800px;
+    min-width: 560px;
 }
 
 .modal-content.full {
     max-width: 95vw;
     max-height: 95vh;
+    min-width: min(95vw, 700px);
 }
 
 /* ============================================

src/utils/ModalManager.js

@@ -124,8 +124,12 @@ class ModalManager {
         // Track open modals
         ModalManager.#openModals.push(this);
 
-        // Show modal (remove hidden, add open)
+        // Show modal: remove hidden first, force a reflow so the browser
+        // records opacity:0 as the start state, then add 'open' to trigger
+        // the CSS transition. Without the reflow, both class changes are
+        // batched into one style recalculation and the transition is skipped.
         this.#modal.classList.remove('hidden');
+        void this.#modal.offsetHeight; // Force reflow — do not remove
         this.#modal.classList.add('open');
         this.#isOpen = true;
 
@@ -165,10 +169,28 @@ class ModalManager {
             ModalManager.#openModals.splice(index, 1);
         }
 
-        // Hide modal
+        // Start hide transition
         this.#modal.classList.remove('open');
         this.#isOpen = false;
 
+        // Re-add 'hidden' after the CSS transition completes so the modal
+        // is fully removed from rendering (display:none), not just invisible.
+        // We use both transitionend and a setTimeout fallback because
+        // transitionend never fires when prefers-reduced-motion disables transitions.
+        let hidden = false;
+        const addHidden = () => {
+            if (hidden || this.#isOpen) return;
+            hidden = true;
+            this.#modal.removeEventListener('transitionend', onTransitionEnd);
+            this.#modal.classList.add('hidden');
+        };
+        const onTransitionEnd = (e) => {
+            if (e.target !== this.#modal) return;
+            addHidden();
+        };
+        this.#modal.addEventListener('transitionend', onTransitionEnd);
+        setTimeout(addHidden, 250); // fallback: slightly longer than 200ms transition
+
         // Restore body scroll if no modals open
         if (ModalManager.#openModals.length === 0) {
             document.body.style.overflow = '';

tests/modal-manager.test.js

@@ -0,0 +1,280 @@
+/**
+ * Tests for ModalManager
+ * Covers the three bugs fixed in modal refactor:
+ *   1. open() animation: reflow between hidden removal and open class add
+ *   2. close() cleanup: 'hidden' class restored after transition
+ *   3. State management: isOpen() accuracy
+ */
+
+const { ModalManager } = require('../src/utils/ModalManager');
+
+function createModalElement(id = 'test-modal') {
+    const modal = document.createElement('div');
+    modal.id = id;
+    modal.className = 'modal hidden';
+    modal.setAttribute('role', 'dialog');
+    modal.setAttribute('aria-modal', 'true');
+
+    const backdrop = document.createElement('div');
+    backdrop.className = 'modal-backdrop';
+    backdrop.setAttribute('data-close', '');
+
+    const content = document.createElement('div');
+    content.className = 'modal-content';
+
+    const header = document.createElement('div');
+    header.className = 'modal-header';
+
+    const closeBtn = document.createElement('button');
+    closeBtn.className = 'modal-close';
+    closeBtn.setAttribute('aria-label', 'Close');
+
+    const body = document.createElement('div');
+    body.className = 'modal-body';
+
+    const input = document.createElement('input');
+    input.type = 'text';
+
+    body.appendChild(input);
+    header.appendChild(closeBtn);
+    content.appendChild(header);
+    content.appendChild(body);
+    modal.appendChild(backdrop);
+    modal.appendChild(content);
+    document.body.appendChild(modal);
+
+    return modal;
+}
+
+describe('ModalManager', () => {
+    let modal;
+    let manager;
+
+    beforeEach(() => {
+        modal = createModalElement();
+        manager = new ModalManager(modal);
+    });
+
+    afterEach(() => {
+        manager.destroy();
+        while (document.body.firstChild) {
+            document.body.removeChild(document.body.firstChild);
+        }
+        document.body.style.overflow = '';
+    });
+
+    // =========================================================
+    // open()
+    // =========================================================
+
+    describe('open()', () => {
+        test('removes hidden class and adds open class', () => {
+            expect(modal.classList.contains('hidden')).toBe(true);
+            expect(modal.classList.contains('open')).toBe(false);
+
+            manager.open();
+
+            expect(modal.classList.contains('hidden')).toBe(false);
+            expect(modal.classList.contains('open')).toBe(true);
+        });
+
+        test('sets isOpen to true', () => {
+            expect(manager.isOpen()).toBe(false);
+            manager.open();
+            expect(manager.isOpen()).toBe(true);
+        });
+
+        test('prevents body scroll', () => {
+            manager.open();
+            expect(document.body.style.overflow).toBe('hidden');
+        });
+
+        test('does not open again if already open', () => {
+            manager.open();
+            manager.open(); // second call should be no-op
+            expect(manager.isOpen()).toBe(true);
+        });
+
+        test('calls onOpen callback', () => {
+            const onOpen = jest.fn();
+            manager.destroy();
+            manager = new ModalManager(modal, { onOpen });
+            manager.open();
+            expect(onOpen).toHaveBeenCalledTimes(1);
+        });
+
+        test('dispatches modal:open custom event', () => {
+            const handler = jest.fn();
+            modal.addEventListener('modal:open', handler);
+            manager.open();
+            expect(handler).toHaveBeenCalledTimes(1);
+        });
+    });
+
+    // =========================================================
+    // close()
+    // =========================================================
+
+    describe('close()', () => {
+        beforeEach(() => {
+            manager.open();
+        });
+
+        test('removes open class', () => {
+            expect(modal.classList.contains('open')).toBe(true);
+            manager.close();
+            expect(modal.classList.contains('open')).toBe(false);
+        });
+
+        test('sets isOpen to false immediately', () => {
+            manager.close();
+            expect(manager.isOpen()).toBe(false);
+        });
+
+        test('restores body scroll when no modals remain open', () => {
+            manager.close();
+            expect(document.body.style.overflow).toBe('');
+        });
+
+        test('adds hidden class after transitionend event fires', () => {
+            manager.close();
+
+            // Immediately after close(): hidden should NOT yet be added —
+            // the close animation is still in progress.
+            // (This is the bug that existed before the fix.)
+            expect(modal.classList.contains('hidden')).toBe(false);
+
+            // Simulate the CSS transition completing
+            const event = new Event('transitionend');
+            Object.defineProperty(event, 'target', { value: modal, writable: false });
+            modal.dispatchEvent(event);
+
+            expect(modal.classList.contains('hidden')).toBe(true);
+        });
+
+        test('adds hidden class via 250ms timeout fallback when transitionend never fires', () => {
+            jest.useFakeTimers();
+
+            manager.close();
+            expect(modal.classList.contains('hidden')).toBe(false);
+
+            // Advance past the fallback timeout (250ms)
+            jest.advanceTimersByTime(300);
+
+            expect(modal.classList.contains('hidden')).toBe(true);
+
+            jest.useRealTimers();
+        });
+
+        test('does not add hidden class if modal is reopened before timeout fires', () => {
+            jest.useFakeTimers();
+
+            manager.close();
+            jest.advanceTimersByTime(100); // halfway through timeout
+
+            // Re-open the modal before the timeout fires
+            manager.open();
+
+            jest.advanceTimersByTime(200); // past original timeout expiry
+
+            // Modal was reopened, so hidden must NOT have been added
+            expect(modal.classList.contains('hidden')).toBe(false);
+            expect(modal.classList.contains('open')).toBe(true);
+
+            jest.useRealTimers();
+        });
+
+        test('calls onClose callback', () => {
+            const onClose = jest.fn();
+            manager.destroy();
+            manager = new ModalManager(modal, { onClose });
+            manager.open();
+            manager.close();
+            expect(onClose).toHaveBeenCalledTimes(1);
+        });
+
+        test('dispatches modal:close custom event', () => {
+            const handler = jest.fn();
+            modal.addEventListener('modal:close', handler);
+            manager.close();
+            expect(handler).toHaveBeenCalledTimes(1);
+        });
+
+        test('is a no-op when modal is already closed', () => {
+            manager.close(); // close from open
+            const onClose = jest.fn();
+            manager.destroy();
+            manager = new ModalManager(modal, { onClose });
+            manager.close(); // call close on an already-closed modal
+            expect(onClose).not.toHaveBeenCalled();
+        });
+    });
+
+    // =========================================================
+    // Keyboard interaction
+    // =========================================================
+
+    describe('keyboard shortcuts', () => {
+        test('Escape key closes an open modal', () => {
+            manager.open();
+            document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
+            expect(manager.isOpen()).toBe(false);
+        });
+
+        test('Escape key does nothing when modal is already closed', () => {
+            expect(() => {
+                document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
+            }).not.toThrow();
+        });
+
+        test('closeOnEscape: false prevents Escape from closing', () => {
+            manager.destroy();
+            manager = new ModalManager(modal, { closeOnEscape: false });
+            manager.open();
+            document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
+            expect(manager.isOpen()).toBe(true);
+        });
+    });
+
+    // =========================================================
+    // Close triggers
+    // =========================================================
+
+    describe('close triggers', () => {
+        test('clicking the × close button closes the modal', () => {
+            manager.open();
+            modal.querySelector('.modal-close').click();
+            expect(manager.isOpen()).toBe(false);
+        });
+
+        test('clicking backdrop (data-close) closes the modal', () => {
+            manager.open();
+            modal.querySelector('.modal-backdrop').click();
+            expect(manager.isOpen()).toBe(false);
+        });
+
+        test('closeOnBackdrop: false prevents backdrop from closing', () => {
+            manager.destroy();
+            manager = new ModalManager(modal, { closeOnBackdrop: false });
+            manager.open();
+            modal.querySelector('.modal-backdrop').click();
+            expect(manager.isOpen()).toBe(true);
+        });
+    });
+
+    // =========================================================
+    // destroy()
+    // =========================================================
+
+    describe('destroy()', () => {
+        test('closes modal if open', () => {
+            manager.open();
+            manager.destroy();
+            expect(manager.isOpen()).toBe(false);
+        });
+
+        test('does not throw when destroying a closed modal', () => {
+            expect(() => manager.destroy()).not.toThrow();
+        });
+    });
+});