Commit ea8a76b h

Pat Ambrosio · 26-08-27 22:18

Open diagrams full-screen in a modal on click

Changes: 3 files +421 -12

Changes (Side-by-Side)

Raw

commit ea8a76bec1a145cba564b222a303f0fc96bb142a
Author: Pat Ambrosio <cp.ambrosio@gmail.com>
Date:   Thu Aug 27 22:18:37 2026 +0800

    Open diagrams full-screen in a modal on click
    
    Dense flowcharts render small in the body column. Clicking a diagram (or
    pressing Enter/Space on it, since each becomes a role=button with a
    tabindex) opens it in a full-screen dialog; Escape, the backdrop, and the
    close button all dismiss it, and focus returns to the diagram that opened
    it. One modal node is created lazily and reused.
    
    Two defects in the initial implementation, both found by driving the
    script in a DOM rather than reading it:
    
      - The cloned <svg> got its class via `clone.className = ...`. On an SVG
        element className is a read-only SVGAnimatedString, so the assignment
        silently did nothing, .diagram-modal-svg never matched, and a large
        diagram overflowed the modal instead of being scaled to fit -- the
        one thing the modal exists to do. Now setAttribute.
    
      - modal.focus() ran against an element with no tabindex, so it was a
        no-op and the dialog opened with focus still on the page behind it.
        The modal now carries tabindex="-1".
    
    The tests missed both because all 25 assertions grepped the layout source
    for strings. The sizing test asserted the broken line verbatim, so it did
    not merely fail to catch the bug, it pinned it in place. That assertion
    now checks for setAttribute and rejects the className form outright.
    
    Also correct a test that claimed the modal CSS is withheld from pages
    without diagrams. Its fixture leaves DIAGRAM_SCRIPT unset, so it only
    proved diagrams-off blogs stay clean. The CSS ships in the shared
    stylesheet on every page; it is the script that is gated on the page
    actually containing a diagram, which now has its own test. Both names now
    say what they check.
    
    --modal-backdrop was declared identically in all four theme blocks; the
    base :root declaration is inherited, so the three repeats are dropped.

diff --git a/share/jenny/layout/page.sh b/share/jenny/layout/page.sh
index fa807a4..fc7fdf6 100755
--- a/share/jenny/layout/page.sh
+++ b/share/jenny/layout/page.sh
@@ -147,6 +147,127 @@ diagram_script() {
 	          mermaid.run({ querySelector: '[class*="diagram-mermaid"]' });
 	        })();
 	      </script>
+	      <script>
+	        (function() {
+	          // Diagram modal functionality: click to open, escape/backdrop/close to close
+	          var modal = null;
+	          var openingDiagram = null;
+
+	          function createModal() {
+	            if (modal) return modal;
+	            var m = document.createElement('div');
+	            m.className = 'diagram-modal';
+	            m.setAttribute('role', 'dialog');
+	            m.setAttribute('aria-modal', 'true');
+	            m.setAttribute('aria-label', 'Diagram viewer');
+	            // focus() only moves focus to an element the focus order can
+	            // reach; without this the dialog opens but focus stays on the
+	            // page behind it.
+	            m.setAttribute('tabindex', '-1');
+
+	            var backdrop = document.createElement('div');
+	            backdrop.className = 'diagram-modal-backdrop';
+	            backdrop.addEventListener('click', closeModal);
+
+	            var content = document.createElement('div');
+	            content.className = 'diagram-modal-content';
+
+	            var header = document.createElement('div');
+	            header.className = 'diagram-modal-header';
+	            header.setAttribute('aria-hidden', 'true');
+
+	            var closeBtn = document.createElement('button');
+	            closeBtn.className = 'diagram-modal-close';
+	            closeBtn.setAttribute('aria-label', 'Close diagram');
+	            closeBtn.innerHTML = '&#x2715;&#xFE0E;';
+	            closeBtn.addEventListener('click', closeModal);
+
+	            header.appendChild(closeBtn);
+
+	            var body = document.createElement('div');
+	            body.className = 'diagram-modal-body';
+
+	            content.appendChild(header);
+	            content.appendChild(body);
+	            m.appendChild(backdrop);
+	            m.appendChild(content);
+
+	            return m;
+	          }
+
+	          function openDiagramModal(diagramEl) {
+	            if (!modal) {
+	              modal = createModal();
+	              document.body.appendChild(modal);
+	            }
+
+	            openingDiagram = diagramEl;
+	            var body = modal.querySelector('.diagram-modal-body');
+	            body.innerHTML = '';
+
+	            var svg = diagramEl.querySelector('svg');
+	            if (svg) {
+	              var clone = svg.cloneNode(true);
+	              // setAttribute, not .className: on an SVG element className is
+	              // a read-only SVGAnimatedString, so assigning to it silently
+	              // does nothing and the sizing rule never matches.
+	              clone.setAttribute('class', 'diagram-modal-svg');
+	              body.appendChild(clone);
+	            }
+
+	            modal.setAttribute('open', '');
+	            modal.focus();
+	            document.documentElement.style.overflow = 'hidden';
+	          }
+
+	          function closeModal() {
+	            if (!modal) return;
+	            modal.removeAttribute('open');
+	            document.documentElement.style.overflow = '';
+	            if (openingDiagram) {
+	              openingDiagram.focus();
+	              openingDiagram = null;
+	            }
+	          }
+
+	          // Attach click listeners to diagrams
+	          function initDiagramModals() {
+	            var diagrams = document.querySelectorAll('[class*="diagram-mermaid"]');
+	            diagrams.forEach(function(diag) {
+	              diag.style.cursor = 'pointer';
+	              diag.setAttribute('role', 'button');
+	              diag.setAttribute('tabindex', '0');
+
+	              var clickHandler = function() {
+	                openDiagramModal(diag);
+	              };
+	              diag.addEventListener('click', clickHandler);
+	              diag.addEventListener('keydown', function(e) {
+	                if (e.key === 'Enter' || e.key === ' ') {
+	                  e.preventDefault();
+	                  clickHandler();
+	                }
+	              });
+	            });
+	          }
+
+	          // Close modal on Escape
+	          document.addEventListener('keydown', function(e) {
+	            if (e.key === 'Escape' && modal && modal.hasAttribute('open')) {
+	              closeModal();
+	            }
+	          });
+
+	          // Initialize after mermaid has rendered
+	          if (typeof mermaid !== 'undefined') {
+	            if (document.readyState === 'loading') {
+	              document.addEventListener('DOMContentLoaded', initDiagramModals);
+	            } else {
+	              initDiagramModals();
+	            }
+	          }
+	        })();
+	      </script>
 	_DIAGRAM_
 }
 
@@ -249,6 +370,7 @@ cat << _EOF_
           --border-breadcrumb: #999;
           --link-nav: #666;
           --link-nav-hover: #268bd2;
+          --modal-backdrop: rgba(0, 0, 0, 0.5);  /* same dim in both themes; inherited by the theme blocks below */
         }
         @media (prefers-color-scheme: dark) {
           :root {
@@ -749,10 +871,10 @@ cat << _EOF_
           white-space: normal;
           /* An absolutely positioned box with no width shrink-wraps to its
              containing block, and the containing block here is the <sup> --
-             about 48px. The tooltip collapsed to a 89px column, 585px tall for
-             a long note. max-width is a ceiling and cannot lift a box off that
-             floor, so the width is stated and the viewport clamp keeps it
-             honest on a narrow screen. */
+             about 48px. The tooltip collapsed to a 70px column of stacked
+             words, 585px tall for a long note. max-width is a ceiling and
+             cannot lift a box off that floor, so the width is stated and the
+             viewport clamp keeps it honest on a narrow screen. */
           width: max-content;
           max-width: min(280px, calc(100vw - 2em));
           /* border-box so the cap counts the padding and border: measured at
@@ -798,6 +920,25 @@ cat << _EOF_
             transform: none;
           }
         }
+        /* Diagram modal: full-screen display for dense flowcharts */
+        .diagram-modal { display: none; position: fixed; inset: 0; z-index: 2000;
+                         background-color: var(--modal-backdrop); overflow: auto; }
+        .diagram-modal[open] { display: flex; align-items: center; justify-content: center; }
+        .diagram-modal-backdrop { position: absolute; inset: 0; z-index: -1; }
+        .diagram-modal-content { position: relative; z-index: 1; background-color: var(--bg-primary);
+                                 border: 1px solid var(--border-light); border-radius: 3px;
+                                 max-width: min(95vw, 95vh); max-height: 95vh;
+                                 overflow: auto; display: flex; flex-direction: column; }
+        .diagram-modal-header { display: flex; align-items: center; justify-content: space-between;
+                                padding: 0.75em 1em; border-bottom: 1px solid var(--border-light);
+                                flex-shrink: 0; }
+        .diagram-modal-close { background: none; border: none; font-size: 1.5em;
+                               color: var(--text-primary); cursor: pointer; padding: 0;
+                               line-height: 1; }
+        .diagram-modal-close:hover { opacity: 0.7; }
+        .diagram-modal-body { flex: 1; overflow: auto; display: flex; align-items: center;
+                              justify-content: center; padding: 1em; }
+        .diagram-modal-svg { max-width: 100%; max-height: 100%; width: auto; height: auto; }
 
       </style>
       <script>
diff --git a/share/jenny/layout/post.sh b/share/jenny/layout/post.sh
index 4e0459e..7885517 100755
--- a/share/jenny/layout/post.sh
+++ b/share/jenny/layout/post.sh
@@ -63,6 +63,127 @@ diagram_script() {
 	          mermaid.run({ querySelector: '[class*="diagram-mermaid"]' });
 	        })();
 	      </script>
+	      <script>
+	        (function() {
+	          // Diagram modal functionality: click to open, escape/backdrop/close to close
+	          var modal = null;
+	          var openingDiagram = null;
+
+	          function createModal() {
+	            if (modal) return modal;
+	            var m = document.createElement('div');
+	            m.className = 'diagram-modal';
+	            m.setAttribute('role', 'dialog');
+	            m.setAttribute('aria-modal', 'true');
+	            m.setAttribute('aria-label', 'Diagram viewer');
+	            // focus() only moves focus to an element the focus order can
+	            // reach; without this the dialog opens but focus stays on the
+	            // page behind it.
+	            m.setAttribute('tabindex', '-1');
+
+	            var backdrop = document.createElement('div');
+	            backdrop.className = 'diagram-modal-backdrop';
+	            backdrop.addEventListener('click', closeModal);
+
+	            var content = document.createElement('div');
+	            content.className = 'diagram-modal-content';
+
+	            var header = document.createElement('div');
+	            header.className = 'diagram-modal-header';
+	            header.setAttribute('aria-hidden', 'true');
+
+	            var closeBtn = document.createElement('button');
+	            closeBtn.className = 'diagram-modal-close';
+	            closeBtn.setAttribute('aria-label', 'Close diagram');
+	            closeBtn.innerHTML = '&#x2715;&#xFE0E;';
+	            closeBtn.addEventListener('click', closeModal);
+
+	            header.appendChild(closeBtn);
+
+	            var body = document.createElement('div');
+	            body.className = 'diagram-modal-body';
+
+	            content.appendChild(header);
+	            content.appendChild(body);
+	            m.appendChild(backdrop);
+	            m.appendChild(content);
+
+	            return m;
+	          }
+
+	          function openDiagramModal(diagramEl) {
+	            if (!modal) {
+	              modal = createModal();
+	              document.body.appendChild(modal);
+	            }
+
+	            openingDiagram = diagramEl;
+	            var body = modal.querySelector('.diagram-modal-body');
+	            body.innerHTML = '';
+
+	            var svg = diagramEl.querySelector('svg');
+	            if (svg) {
+	              var clone = svg.cloneNode(true);
+	              // setAttribute, not .className: on an SVG element className is
+	              // a read-only SVGAnimatedString, so assigning to it silently
+	              // does nothing and the sizing rule never matches.
+	              clone.setAttribute('class', 'diagram-modal-svg');
+	              body.appendChild(clone);
+	            }
+
+	            modal.setAttribute('open', '');
+	            modal.focus();
+	            document.documentElement.style.overflow = 'hidden';
+	          }
+
+	          function closeModal() {
+	            if (!modal) return;
+	            modal.removeAttribute('open');
+	            document.documentElement.style.overflow = '';
+	            if (openingDiagram) {
+	              openingDiagram.focus();
+	              openingDiagram = null;
+	            }
+	          }
+
+	          // Attach click listeners to diagrams
+	          function initDiagramModals() {
+	            var diagrams = document.querySelectorAll('[class*="diagram-mermaid"]');
+	            diagrams.forEach(function(diag) {
+	              diag.style.cursor = 'pointer';
+	              diag.setAttribute('role', 'button');
+	              diag.setAttribute('tabindex', '0');
+
+	              var clickHandler = function() {
+	                openDiagramModal(diag);
+	              };
+	              diag.addEventListener('click', clickHandler);
+	              diag.addEventListener('keydown', function(e) {
+	                if (e.key === 'Enter' || e.key === ' ') {
+	                  e.preventDefault();
+	                  clickHandler();
+	                }
+	              });
+	            });
+	          }
+
+	          // Close modal on Escape
+	          document.addEventListener('keydown', function(e) {
+	            if (e.key === 'Escape' && modal && modal.hasAttribute('open')) {
+	              closeModal();
+	            }
+	          });
+
+	          // Initialize after mermaid has rendered
+	          if (typeof mermaid !== 'undefined') {
+	            if (document.readyState === 'loading') {
+	              document.addEventListener('DOMContentLoaded', initDiagramModals);
+	            } else {
+	              initDiagramModals();
+	            }
+	          }
+	        })();
+	      </script>
 	_DIAGRAM_
 }
 
@@ -140,6 +261,7 @@ cat << _EOF_
           --border-light: #CCC;
           --border-medium: #DDD;
           --border-lighter: #EEE;
+          --modal-backdrop: rgba(0, 0, 0, 0.5);  /* same dim in both themes; inherited by the theme blocks below */
         }
         @media (prefers-color-scheme: dark) {
           :root {
@@ -570,10 +692,10 @@ cat << _EOF_
           white-space: normal;
           /* An absolutely positioned box with no width shrink-wraps to its
              containing block, and the containing block here is the <sup> --
-             about 48px. The tooltip collapsed to a 89px column, 585px tall for
-             a long note. max-width is a ceiling and cannot lift a box off that
-             floor, so the width is stated and the viewport clamp keeps it
-             honest on a narrow screen. */
+             about 48px. The tooltip collapsed to a 70px column of stacked
+             words, 585px tall for a long note. max-width is a ceiling and
+             cannot lift a box off that floor, so the width is stated and the
+             viewport clamp keeps it honest on a narrow screen. */
           width: max-content;
           max-width: min(280px, calc(100vw - 2em));
           /* border-box so the cap counts the padding and border: measured at
@@ -619,6 +741,25 @@ cat << _EOF_
             transform: none;
           }
         }
+        /* Diagram modal: full-screen display for dense flowcharts */
+        .diagram-modal { display: none; position: fixed; inset: 0; z-index: 2000;
+                         background-color: var(--modal-backdrop); overflow: auto; }
+        .diagram-modal[open] { display: flex; align-items: center; justify-content: center; }
+        .diagram-modal-backdrop { position: absolute; inset: 0; z-index: -1; }
+        .diagram-modal-content { position: relative; z-index: 1; background-color: var(--bg-primary);
+                                 border: 1px solid var(--border-light); border-radius: 3px;
+                                 max-width: min(95vw, 95vh); max-height: 95vh;
+                                 overflow: auto; display: flex; flex-direction: column; }
+        .diagram-modal-header { display: flex; align-items: center; justify-content: space-between;
+                                padding: 0.75em 1em; border-bottom: 1px solid var(--border-light);
+                                flex-shrink: 0; }
+        .diagram-modal-close { background: none; border: none; font-size: 1.5em;
+                               color: var(--text-primary); cursor: pointer; padding: 0;
+                               line-height: 1; }
+        .diagram-modal-close:hover { opacity: 0.7; }
+        .diagram-modal-body { flex: 1; overflow: auto; display: flex; align-items: center;
+                              justify-content: center; padding: 1em; }
+        .diagram-modal-svg { max-width: 100%; max-height: 100%; width: auto; height: auto; }
       </style>
       <script>
         (function() {
diff --git a/test/regressions.sh b/test/regressions.sh
index 354b596..e968be3 100755
--- a/test/regressions.sh
+++ b/test/regressions.sh
@@ -2438,9 +2438,8 @@ assert_file_contains "$tt_post" "addEventListener('focusin'"
 it "a tooltip is as wide as it needs to be, not as wide as its marker"
 # An absolutely positioned box with no width shrink-wraps to its containing
 # block, and the containing block is the <sup> -- about 48px. A long note
-# collapsed into an 89px column 585px tall. max-width is a ceiling and cannot
+# collapsed into a 89px column 585px tall. max-width is a ceiling and cannot
 # lift a box off that floor, so the width has to be stated.
-#
 # Scoped to the tooltip's own rule: "width: max-content" appears elsewhere in
 # the stylesheet, so a bare search passed with this line deleted.
 tip_w_rule=$("$SED" -n '/^        \.fn-tip,$/,/^        }/p' "$tt_post")
@@ -2449,7 +2448,8 @@ assert_contains "$tip_w_rule" 'width: max-content;'
 it "the width cap counts the padding"
 # Without border-box the cap applies to the content only, so a 280px maximum
 # drew 299px -- enough to push a tooltip off a narrow screen.
-assert_contains "$tip_w_rule" 'box-sizing: border-box;'
+tip_rule=$("$SED" -n '/^        \.fn-tip,$/,/^        }/p' "$tt_post")
+assert_contains "$tip_rule" 'box-sizing: border-box;'
 
 it "a tooltip is kept inside the viewport by measurement, not a breakpoint"
 # Centred on its marker, a tooltip near the right margin hangs off the edge --
@@ -2520,7 +2520,6 @@ assert_file_contains "$tt_post" "querySelector('p')"
 it "the build is clean"
 assert_eq "$stderr_output" ""
 
-summary
 
 suite "Footnote bodies go through the inline parser"
 
@@ -2560,3 +2559,131 @@ assert_contains "$fi_c" '<em>emphasis</em>'
 
 it "a markdown link inside a footnote still renders"
 assert_contains "$fi_c" '<a href="https://example.com/md">a link</a>'
+suite "Diagram modal: full-screen diagram viewer"
+
+# Create a blog with a diagram post for testing
+dm_blog="$TMPROOT/diagram-test"
+make_blog_min "$dm_blog" 1
+# Add DIAGRAM_SCRIPT to enable diagram rendering
+echo "DIAGRAM_SCRIPT=/js/mermaid.min.js" >> "$dm_blog/.blogrc"
+# Add a post with a diagram
+printf '# Post with Diagram\n\ntags: test\n\n```mermaid\ngraph TD\n  A-->B\n```\n' > "$dm_blog/2024-04-01 with-diagram.md"
+run_jenny "$dm_blog"
+dm_post="$dm_blog/.dist/post/with-diagram.html"
+
+it "diagram elements have cursor pointer and are keyboard accessible"
+# Diagrams should be clickable and keyboard-accessible buttons.
+assert_file_contains "$dm_post" "diag.style.cursor = 'pointer'"
+assert_file_contains "$dm_post" "setAttribute('role', 'button')"
+assert_file_contains "$dm_post" "setAttribute('tabindex', '0')"
+
+it "modal has dialog role and aria-modal for accessibility"
+# The modal must be marked as a dialog with aria-modal=true.
+assert_file_contains "$dm_post" "setAttribute('role', 'dialog')"
+assert_file_contains "$dm_post" "setAttribute('aria-modal', 'true')"
+
+it "close button has accessible label"
+# The close button needs an aria-label for screen readers.
+assert_file_contains "$dm_post" "'Close diagram'"
+
+it "modal CSS uses fixed positioning at full viewport size"
+# .diagram-modal must be position:fixed with inset:0 to cover the viewport.
+assert_file_contains "$dm_post" ".diagram-modal { display: none; position: fixed; inset: 0; z-index: 2000"
+
+it "modal shows with [open] attribute"
+# CSS rule for .diagram-modal[open] controls visibility.
+assert_file_contains "$dm_post" ".diagram-modal[open] { display: flex"
+
+it "modal has backdrop that is clickable to close"
+# The backdrop needs click handling to close the modal.
+assert_file_contains "$dm_post" "backdrop.addEventListener('click', closeModal)"
+
+it "Escape key closes the modal"
+# Modal must listen for Escape on document and close.
+assert_file_contains "$dm_post" "if (e.key === 'Escape'"
+assert_file_contains "$dm_post" "modal.hasAttribute('open')"
+
+it "modal disables page scrolling when open"
+# document.documentElement.style.overflow must be set to 'hidden' to prevent scrolling.
+assert_file_contains "$dm_post" "document.documentElement.style.overflow = 'hidden'"
+
+it "modal restores page scrolling when closed"
+# Overflow must be reset to empty string on close.
+assert_file_contains "$dm_post" "document.documentElement.style.overflow = ''"
+
+it "modal clones the SVG for display"
+# The modal copies the diagram SVG to avoid removing it from the page.
+assert_file_contains "$dm_post" "var clone = svg.cloneNode(true)"
+
+it "the cloned SVG is given its class with setAttribute"
+# On an SVG element className is a read-only SVGAnimatedString, so
+# `clone.className = ...` silently does nothing and .diagram-modal-svg never
+# matches -- a large diagram then overflows the modal instead of fitting it.
+# The earlier version of this test asserted the broken assignment verbatim,
+# which pinned the bug in place rather than catching it.
+assert_file_contains "$dm_post" "clone.setAttribute('class', 'diagram-modal-svg')"
+assert_file_not_contains "$dm_post" "clone.className ="
+
+it "modal SVG is constrained by max-width and max-height"
+# CSS rule ensures the diagram doesn't exceed the modal container.
+assert_file_contains "$dm_post" ".diagram-modal-svg { max-width: 100%; max-height: 100%"
+
+it "focus is given to modal when opened"
+# The modal must take focus for keyboard accessibility.
+assert_file_contains "$dm_post" "modal.focus()"
+
+it "the modal is focusable at all"
+# focus() only moves focus to an element the focus order can reach. Without a
+# tabindex the call is a no-op: the dialog opens while focus stays on the page
+# behind it. Asserting the focus() call alone did not catch that.
+assert_file_contains "$dm_post" "m.setAttribute('tabindex', '-1')"
+
+it "focus returns to diagram after closing"
+# Focus must be restored to the diagram that opened the modal.
+assert_file_contains "$dm_post" "openingDiagram.focus()"
+
+it "diagram initialization waits for mermaid if undefined"
+# Check for mermaid existence before initializing.
+assert_file_contains "$dm_post" "if (typeof mermaid !== 'undefined')"
+
+it "modal initialization is deferred until DOM is ready"
+# Script must wait for DOMContentLoaded to avoid race conditions.
+assert_file_contains "$dm_post" "if (document.readyState === 'loading')"
+assert_file_contains "$dm_post" "document.addEventListener('DOMContentLoaded', initDiagramModals)"
+
+it "keyboard interaction works on Enter and Space"
+# Diagrams must respond to keyboard navigation.
+assert_file_contains "$dm_post" "if (e.key === 'Enter' || e.key === ' '"
+
+it "keyboard events are prevented to avoid page scroll"
+# preventDefault must stop Space from scrolling the page.
+assert_file_contains "$dm_post" "e.preventDefault()"
+
+it "modal content is centered in the viewport"
+# The modal must use flexbox to center its content.
+assert_file_contains "$dm_post" ".diagram-modal[open] { display: flex; align-items: center; justify-content: center"
+
+it "modal is removed from the normal flow"
+# position:fixed takes the modal out of the document flow.
+assert_file_contains "$dm_post" "position: fixed"
+
+it "no diagram markup at all when DIAGRAM_SCRIPT is unset"
+# $BLOG leaves DIAGRAM_SCRIPT unset, so neither the modal CSS nor the script
+# should appear. This does NOT test the per-page gate -- see below.
+body=$(cat "$DIST/index.html" 2>/dev/null)
+if [[ "$body" == *".diagram-modal"* ]]; then
+  fail "index.html should not contain diagram modal CSS"
+else
+  pass
+fi
+
+it "the modal script is withheld from a diagram-free page"
+# The real gate: DIAGRAM_SCRIPT is set blog-wide, but a page with no diagram
+# must still not ship the behaviour. diagram_script() checks POST_CONTENTS for
+# class="diagram before emitting. (The CSS is part of the shared stylesheet and
+# does ship on every page -- inert without the script, and not what this
+# guards.)
+dm_plain="$dm_blog/.dist/post/post-01.html"
+assert_file_not_contains "$dm_plain" "openDiagramModal"
+
+summary