From e9504efa24148152cce3bf2bf3f588993d664b80 Mon Sep 17 00:00:00 2001
From: vicrodh <103399283+vicrodh@users.noreply.github.com>
Date: Tue, 12 May 2026 09:16:34 -0600
Subject: [PATCH] perf(modals): gate 8 always-mounted modals with {#if}, drop
 blur shadow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two perf agents independently identified the same dominant cost behind
the 2-3s modal-open latency on non-maximized windows: 8 modals declared
at the top of `+page.svelte` (PlaylistModal, PlaylistImportModal,
FolderEditModal, AboutModal, QualityFallbackModal, KeyboardShortcuts,
KeybindingsSettings, LinkResolverModal) were always mounted, with only
their inner templates gated by `{#if isOpen}`. Their `<script>` blocks
ran for the lifetime of the app and their reactive declarations
re-evaluated on every parent tick — and the parent ticks at ~4Hz during
playback via `playerStore.notifyListeners()`.

PlaylistModal alone is 1,420 lines / 18 reactive declarations. Sum
across 8 modals × ~4Hz × the +page.svelte reactive graph (188+ $state)
explains why mounting any *new* component (Discovery's customize modal,
the back-to-top button) raced with reactive churn and landed seconds
after the user clicked.

Fix: wrap each modal at the mount site in `{#if isOpen}`. The inner
component instance now only exists while the modal is open. Two-line
change per modal, zero API change. The `isOpen` prop on each modal is
left in place so the modal's internal close logic (focus mgmt, ARIA,
close animation) keeps working unchanged.

Also drops the 64px Gaussian box-shadow on `ModalLite.svelte` that I'd
added one commit prior. Under software compositing the blur radius
re-rasterizes whenever anything beneath repaints, defeating the whole
point of removing the overlay. Replaced with a flat composite shadow
(1px edge + 16px small-offset) — reads as "floating" without the
per-frame Gaussian cost.

Documents the pattern in `qbz-nix-docs/qbz-adr/ADR-003-conditional-
mount-pattern.md` so the same anti-pattern can be swept across the rest
of the codebase systematically. The ADR identifies search heuristic
(`<X isOpen={...} />` outside `{#if}`), the cost mechanism, and the
roadmap touchpoints (ModalRouter extraction from +page.svelte) where
the rule should be applied during decomposition.

cc
---
 src/lib/discovery-v2/ModalLite.svelte | 148 +++++++++++++++-----------
 src/routes/+page.svelte               | 138 +++++++++++++-----------
 2 files changed, 161 insertions(+), 125 deletions(-)

diff --git a/src/lib/discovery-v2/ModalLite.svelte b/src/lib/discovery-v2/ModalLite.svelte
index a1b38ac03..b7dcfc628 100644
--- a/src/lib/discovery-v2/ModalLite.svelte
+++ b/src/lib/discovery-v2/ModalLite.svelte
@@ -24,29 +24,58 @@
   }: Props = $props();
 
   /**
-   * Discovery V2 modal — clean-room replacement for `Modal.svelte`.
+   * Discovery V2 modal — overlay-less variant.
    *
-   * The shared modal's slide-up `transform: translateY+scale` animation
-   * is GPU-friendly at maximize (WebKitGTK promotes the modal to a GPU
-   * layer) but software-rasterized at smaller windows, where the user
-   * reported visible render delays. Scale animations under software comp
-   * require per-pixel interpolation every frame on a ~480x600 surface.
+   * Earlier attempts kept a full-viewport `rgba(0,0,0,0.7)` scrim under
+   * the modal for visual focus. The shared `Modal.svelte` does the same.
+   * On WebKitGTK under software compositing (any non-maximized window),
+   * adding that fixed-position scrim forced an extra paint pass against
+   * the underlying DiscoveryView every frame — the user reported 2-3s
+   * to render even with no animation. `will-change: opacity` + GPU layer
+   * hint helped some (3s → 2s) but didn't fix it.
    *
-   * This variant uses opacity-only animation (cheap in either path) and
-   * drops the scale + translate. Same `isOpen / onClose / title /
-   * maxWidth / children / footer` Props as the shared modal so it's a
-   * drop-in for callers. No backdrop-filter on the overlay (same
-   * decision as the perf cleanup we applied to the shared Modal).
+   * This variant drops the scrim entirely. The modal floats above the
+   * page with a heavy border + box-shadow doing the focus work that the
+   * scrim used to. Click-outside-to-close is bound on `document` so it
+   * still works even without an overlay element to absorb the click.
+   * ESC still closes via window keydown.
+   *
+   * Trade-off: less visual emphasis on the modal (background isn't
+   * darkened). Gain: instant mount under software compositing because
+   * there's no full-viewport rect to paint against.
    */
 
-  function handleBackdropClick(e: MouseEvent) {
-    if (e.target === e.currentTarget) onClose();
-  }
+  let modalEl = $state<HTMLDivElement | null>(null);
 
   function handleKeydown(e: KeyboardEvent) {
     if (e.key === 'Escape' && isOpen) onClose();
   }
 
+  function handleDocumentClick(e: MouseEvent) {
+    if (!isOpen) return;
+    if (modalEl && !modalEl.contains(e.target as Node)) onClose();
+  }
+
+  // Document-level outside-click only attaches while the modal is open.
+  // The mousedown phase fires before click, so we use it to avoid the
+  // race where opening the modal via a button-click also fires this
+  // handler (the click event that opened the modal would already have
+  // bubbled to document by the time mousedown registers next).
+  $effect(() => {
+    if (!isOpen) return;
+    // Defer registering until the next frame so the click that opened
+    // the modal doesn't immediately close it on the same event loop.
+    let cancelled = false;
+    requestAnimationFrame(() => {
+      if (cancelled) return;
+      document.addEventListener('mousedown', handleDocumentClick);
+    });
+    return () => {
+      cancelled = true;
+      document.removeEventListener('mousedown', handleDocumentClick);
+    };
+  });
+
   function portal(node: HTMLElement) {
     document.body.appendChild(node);
     return {
@@ -60,74 +89,67 @@
 <svelte:window onkeydown={handleKeydown} />
 
 {#if isOpen}
-  <!-- svelte-ignore a11y_click_events_have_key_events, a11y_interactive_supports_focus -->
   <div
-    class="modal-overlay"
+    class="modal"
     use:portal
-    onclick={handleBackdropClick}
+    bind:this={modalEl}
     role="dialog"
     aria-modal="true"
     tabindex="-1"
+    style="max-width: {maxWidth}"
   >
-    <div class="modal" style="max-width: {maxWidth}">
-      {#if title || showCloseButton}
-        <div class="modal-header">
-          {#if title}
-            <h2>{title}</h2>
-          {:else}
-            <div></div>
-          {/if}
-          {#if showCloseButton}
-            <button class="close-btn" type="button" aria-label={$t('actions.close')} onclick={onClose}>
-              <X size={18} />
-            </button>
-          {/if}
-        </div>
-      {/if}
-      <div class="modal-body">
-        {@render children()}
+    {#if title || showCloseButton}
+      <div class="modal-header">
+        {#if title}
+          <h2>{title}</h2>
+        {:else}
+          <div></div>
+        {/if}
+        {#if showCloseButton}
+          <button class="close-btn" type="button" aria-label={$t('actions.close')} onclick={onClose}>
+            <X size={18} />
+          </button>
+        {/if}
       </div>
-      {#if footer}
-        <div class="modal-footer">
-          {@render footer()}
-        </div>
-      {/if}
+    {/if}
+    <div class="modal-body">
+      {@render children()}
     </div>
+    {#if footer}
+      <div class="modal-footer">
+        {@render footer()}
+      </div>
+    {/if}
   </div>
 {/if}
 
 <style>
-  /* Opacity-only fade-in. No transform animation on the modal itself —
-     the shared Modal.svelte's `slide-up` keyframe uses scale + translate,
-     which forces per-frame pixel interpolation under software compositing
-     and made smaller-window modal opens feel slow. Opacity changes are
-     cheap on either path. */
-  .modal-overlay {
-    position: fixed;
-    inset: 0;
-    background: rgba(0, 0, 0, 0.7);
-    display: flex;
-    align-items: center;
-    justify-content: center;
-    padding: 20px;
-    z-index: 200000;
-    animation: modal-fade-in 120ms ease-out;
-  }
-
-  @keyframes modal-fade-in {
-    from { opacity: 0; }
-    to { opacity: 1; }
-  }
-
+  /* Overlay-less floating modal. Centered via fixed positioning + 50/50
+     translate. Border + box-shadow give the modal "lift" without
+     darkening the rest of the viewport. No scrim element means no
+     full-viewport paint pass when the modal mounts — that was the
+     dominant cost on non-maximized windows. */
   .modal {
+    position: fixed;
+    top: 50%;
+    left: 50%;
+    transform: translate(-50%, -50%);
+    width: calc(100% - 40px);
+    max-height: calc(100dvh - 40px);
     background: var(--bg-primary);
     border: 1px solid var(--bg-tertiary);
     border-radius: 12px;
-    width: 100%;
-    max-height: calc(100dvh - 40px);
     display: flex;
     flex-direction: column;
     overflow: hidden;
+    z-index: 200000;
+    /* Flat shadow stack instead of a 64px Gaussian — under software
+       compositing the blur radius dominates paint cost and re-rasterizes
+       whenever anything beneath the modal repaints. A composite of a
+       hairline edge shadow + a small offset shadow reads as "floating"
+       without the expensive blur. */
+    box-shadow: 0 0 0 1px rgba(0, 0, 0, 0.6),
+      0 8px 16px rgba(0, 0, 0, 0.5);
   }
 
   .modal-header {
diff --git a/src/routes/+page.svelte b/src/routes/+page.svelte
index bc7ad1a2b..c9fd9cee5 100644
--- a/src/routes/+page.svelte
+++ b/src/routes/+page.svelte
@@ -6898,75 +6898,89 @@
       />
     {/if}
 
-    <!-- Playlist Modal -->
-    <PlaylistModal
-      isOpen={isPlaylistModalOpen}
-      mode={playlistModalMode}
-      playlist={playlistModalMode === 'edit' ? playlistModalEditPlaylist : undefined}
-      trackIds={playlistModalTrackIds}
-      isLocalTracks={playlistModalTracksAreLocal}
-      plexRatingKeys={playlistModalPlexRatingKeys}
-      isHidden={playlistModalMode === 'edit' ? playlistModalEditIsHidden : false}
-      currentFolderId={playlistModalMode === 'edit' ? playlistModalEditCurrentFolderId : null}
-      {userPlaylists}
-      onClose={handlePlaylistModalClose}
-      onSuccess={handlePlaylistCreated}
-    />
+    <!-- Modals are gated by `{#if}` at the mount site so their <script>
+         blocks (and the reactive declarations within) don't execute when
+         the modal is closed. Previously each modal was always mounted with
+         only its template gated internally, which kept its full reactive
+         graph alive and re-evaluating on every parent tick — the dominant
+         cost of opening any new modal at non-maximized window sizes. -->
+    {#if isPlaylistModalOpen}
+      <PlaylistModal
+        isOpen={isPlaylistModalOpen}
+        mode={playlistModalMode}
+        playlist={playlistModalMode === 'edit' ? playlistModalEditPlaylist : undefined}
+        trackIds={playlistModalTrackIds}
+        isLocalTracks={playlistModalTracksAreLocal}
+        plexRatingKeys={playlistModalPlexRatingKeys}
+        isHidden={playlistModalMode === 'edit' ? playlistModalEditIsHidden : false}
+        currentFolderId={playlistModalMode === 'edit' ? playlistModalEditCurrentFolderId : null}
+        {userPlaylists}
+        onClose={handlePlaylistModalClose}
+        onSuccess={handlePlaylistCreated}
+      />
+    {/if}
 
-    <!-- Playlist Import Modal -->
-    <PlaylistImportModal
-      isOpen={isPlaylistImportOpen}
-      onClose={closePlaylistImport}
-      onSuccess={handlePlaylistImported}
-    />
+    {#if isPlaylistImportOpen}
+      <PlaylistImportModal
+        isOpen={isPlaylistImportOpen}
+        onClose={closePlaylistImport}
+        onSuccess={handlePlaylistImported}
+      />
+    {/if}
 
-    <!-- Folder Edit Modal (sidebar entry-point — issue #364) -->
-    <FolderEditModal
-      isOpen={isSidebarFolderEditOpen}
-      folder={editingSidebarFolder}
-      onClose={closeSidebarFolderEdit}
-      onSave={handleSidebarFolderSave}
-      onDelete={handleSidebarFolderDelete}
-    />
+    {#if isSidebarFolderEditOpen}
+      <FolderEditModal
+        isOpen={isSidebarFolderEditOpen}
+        folder={editingSidebarFolder}
+        onClose={closeSidebarFolderEdit}
+        onSave={handleSidebarFolderSave}
+        onDelete={handleSidebarFolderDelete}
+      />
+    {/if}
 
-    <!-- About Modal -->
-    <AboutModal
-      isOpen={isAboutModalOpen}
-      onClose={() => isAboutModalOpen = false}
-    />
+    {#if isAboutModalOpen}
+      <AboutModal
+        isOpen={isAboutModalOpen}
+        onClose={() => isAboutModalOpen = false}
+      />
+    {/if}
 
-    <!-- Quality Fallback Modal -->
-    <QualityFallbackModal
-      isOpen={isQualityFallbackOpen}
-      trackTitle={qualityFallbackTrackTitle}
-      onTryLower={handleQualityFallbackTryLower}
-      onSkip={handleQualityFallbackSkip}
-      onClose={() => isQualityFallbackOpen = false}
-    />
+    {#if isQualityFallbackOpen}
+      <QualityFallbackModal
+        isOpen={isQualityFallbackOpen}
+        trackTitle={qualityFallbackTrackTitle}
+        onTryLower={handleQualityFallbackTryLower}
+        onSkip={handleQualityFallbackSkip}
+        onClose={() => isQualityFallbackOpen = false}
+      />
+    {/if}
 
-    <!-- Keyboard Shortcuts Modal -->
-    <KeyboardShortcutsModal
-      isOpen={isShortcutsModalOpen}
-      onClose={() => isShortcutsModalOpen = false}
-      onOpenSettings={() => {
-        isShortcutsModalOpen = false;
-        isKeybindingsSettingsOpen = true;
-      }}
-    />
+    {#if isShortcutsModalOpen}
+      <KeyboardShortcutsModal
+        isOpen={isShortcutsModalOpen}
+        onClose={() => isShortcutsModalOpen = false}
+        onOpenSettings={() => {
+          isShortcutsModalOpen = false;
+          isKeybindingsSettingsOpen = true;
+        }}
+      />
+    {/if}
 
-    <!-- Keybindings Settings Modal -->
-    <KeybindingsSettings
-      isOpen={isKeybindingsSettingsOpen}
-      onClose={() => isKeybindingsSettingsOpen = false}
-    />
+    {#if isKeybindingsSettingsOpen}
+      <KeybindingsSettings
+        isOpen={isKeybindingsSettingsOpen}
+        onClose={() => isKeybindingsSettingsOpen = false}
+      />
+    {/if}
 
-    <!-- Link Resolver Modal -->
-    <LinkResolverModal
-      isOpen={isLinkResolverOpen}
-      onClose={() => isLinkResolverOpen = false}
-      onResolve={handleResolvedLink}
-      onOpenImporter={() => { isLinkResolverOpen = false; openPlaylistImport(); }}
-    />
+    {#if isLinkResolverOpen}
+      <LinkResolverModal
+        isOpen={isLinkResolverOpen}
+        onClose={() => isLinkResolverOpen = false}
+        onResolve={handleResolvedLink}
+        onOpenImporter={() => { isLinkResolverOpen = false; openPlaylistImport(); }}
+      />
+    {/if}
 
     {#if updateRelease}
       <UpdateAvailableModal