From 1c456e139d1e39c698351e65da014bf6f43fe713 Mon Sep 17 00:00:00 2001
From: Kevin Heis <heiskr@users.noreply.github.com>
Date: Thu, 18 Apr 2024 08:29:16 -0700
Subject: [PATCH] Make markdown image styling consistent (#50200)

---
 src/content-render/stylesheets/images.scss    | 32 -------------------
 src/content-render/stylesheets/index.scss     |  1 -
 .../unified/wrap-procedural-images.js         |  4 +--
 src/fixtures/tests/images.js                  |  2 +-
 .../MarkdownContent.module.scss               |  1 +
 .../MarkdownContent/stylesheets/images.scss   | 20 ++++++++++++
 6 files changed, 24 insertions(+), 36 deletions(-)
 delete mode 100644 src/content-render/stylesheets/images.scss
 create mode 100644 src/frame/components/ui/MarkdownContent/stylesheets/images.scss

diff --git a/src/content-render/stylesheets/images.scss b/src/content-render/stylesheets/images.scss
deleted file mode 100644
index 352b83908e..0000000000
--- a/src/content-render/stylesheets/images.scss
+++ /dev/null
@@ -1,32 +0,0 @@
-.markdown-body .procedural-image-wrapper {
-  display: block;
-  padding: 1rem 0;
-  margin: 1rem 0;
-  border: none;
-  max-width: calc(100% - 2rem);
-
-  img {
-    width: auto;
-    height: auto;
-    max-height: 32rem;
-    padding: 0;
-    box-shadow: var(--color-shadow-medium);
-  }
-
-  img[src*="https://github.githubassets.com/images/icons/emoji"]
-  {
-    box-shadow: none;
-  }
-}
-
-.markdown-body img {
-  max-height: 32rem;
-  padding: 0;
-}
-
-// make sure images that contain emoji render at the expected size
-.markdown-body img[src*="https://github.githubassets.com/images/icons/emoji"]
-{
-  height: 20px;
-  width: 20px;
-}
diff --git a/src/content-render/stylesheets/index.scss b/src/content-render/stylesheets/index.scss
index 05912bd262..956f1f6c35 100644
--- a/src/content-render/stylesheets/index.scss
+++ b/src/content-render/stylesheets/index.scss
@@ -1,6 +1,5 @@
 @import "annotate.scss";
 @import "heading-links.scss";
-@import "images.scss";
 @import "markdown-overrides.scss";
 @import "syntax-highlighting.scss";
 @import "alerts.scss";
diff --git a/src/content-render/unified/wrap-procedural-images.js b/src/content-render/unified/wrap-procedural-images.js
index a4b22b6abd..59aa138f18 100644
--- a/src/content-render/unified/wrap-procedural-images.js
+++ b/src/content-render/unified/wrap-procedural-images.js
@@ -11,7 +11,7 @@ import { visitParents } from 'unist-util-visit-parents'
  *
  *   <ol>
  *     <li>
- *       <span class="procedural-image-wrapper">
+ *       <div class="procedural-image-wrapper">
  *         <img src="..." />
  *
  *
@@ -33,7 +33,7 @@ function insideOlLi(ancestors) {
 function visitor(node, ancestors) {
   if (insideOlLi(ancestors)) {
     const shallowClone = Object.assign({}, node)
-    shallowClone.tagName = 'span'
+    shallowClone.tagName = 'div'
     shallowClone.properties = { class: 'procedural-image-wrapper' }
     shallowClone.children = [node]
     const parent = ancestors.at(-1)
diff --git a/src/fixtures/tests/images.js b/src/fixtures/tests/images.js
index f2ad27eb95..10ffca612c 100644
--- a/src/fixtures/tests/images.js
+++ b/src/fixtures/tests/images.js
@@ -45,7 +45,7 @@ describe('render Markdown image tags', () => {
   test('image inside a list keeps its span', async () => {
     const $ = await getDOM('/get-started/images/images-in-lists')
 
-    const imageSpan = $('#article-contents > div > ol > li > span.procedural-image-wrapper')
+    const imageSpan = $('#article-contents > div > ol > li > div.procedural-image-wrapper')
     expect(imageSpan.length).toBe(1)
   })
 
diff --git a/src/frame/components/ui/MarkdownContent/MarkdownContent.module.scss b/src/frame/components/ui/MarkdownContent/MarkdownContent.module.scss
index a6c58c92a5..8b556e559a 100644
--- a/src/frame/components/ui/MarkdownContent/MarkdownContent.module.scss
+++ b/src/frame/components/ui/MarkdownContent/MarkdownContent.module.scss
@@ -2,6 +2,7 @@
 @import "./stylesheets/headers.scss";
 @import "./stylesheets/lists.scss";
 @import "./stylesheets/table.scss";
+@import "./stylesheets/images.scss";
 
 .markdownBody {
   :not(h1, h2, h3, h4, h5, h6, [role="tablist"]) {
diff --git a/src/frame/components/ui/MarkdownContent/stylesheets/images.scss b/src/frame/components/ui/MarkdownContent/stylesheets/images.scss
new file mode 100644
index 0000000000..e2a4af3338
--- /dev/null
+++ b/src/frame/components/ui/MarkdownContent/stylesheets/images.scss
@@ -0,0 +1,20 @@
+.markdownBody {
+  // All non-emoji screenshots get the same shadow and height constraint
+  img:not([src*="https://github.githubassets.com/images/icons/emoji"])
+  {
+    max-height: 32rem;
+    box-shadow: var(--color-shadow-medium);
+  }
+
+  // Make sure images that contain emoji render at the expected size
+  img[src*="https://github.githubassets.com/images/icons/emoji"]
+  {
+    height: 20px;
+    width: 20px;
+  }
+
+  // For when writers don't leave an empty line and there's no `p` tag preceding the image
+  :global(.procedural-image-wrapper) {
+    margin: 1rem 0;
+  }
+}