Merge pull request #45 from shsteimer/pipeline-utils

feature: rewrite markup out of pipeline to ensure all block div's contain appropriate paragraph tags
adobe · Mar 12, 2024 · 43fb99c · 43fb99c
2 parents 06f155b + c0bc734
commit 43fb99c
Show file tree

Hide file tree

Showing 5 changed files with 257 additions and 1 deletion.
diff --git a/dist/aem.js b/dist/aem.js
@@ -372,6 +372,48 @@ function decorateTemplateAndTheme() {
   if (theme) addClasses(document.body, theme);
 }
 
+/**
+ * Wrap inline text content of block cells within a <p> tag.
+ * @param {Element} block the block element
+ */
+function wrapTextNodes(block) {
+  const validWrappers = [
+    'P',
+    'PRE',
+    'UL',
+    'OL',
+    'PICTURE',
+    'TABLE',
+    'H1',
+    'H2',
+    'H3',
+    'H4',
+    'H5',
+    'H6',
+  ];
+
+  const wrap = (el) => {
+    const wrapper = document.createElement('p');
+    wrapper.append(...el.childNodes);
+    el.append(wrapper);
+  };
+
+  block.querySelectorAll(':scope > div > div').forEach((blockColumn) => {
+    if (blockColumn.hasChildNodes()) {
+      const hasWrapper = !!blockColumn.firstElementChild
+        && validWrappers.some((tagName) => blockColumn.firstElementChild.tagName === tagName);
+      if (!hasWrapper) {
+        wrap(blockColumn);
+      } else if (
+        blockColumn.firstElementChild.tagName === 'PICTURE'
+        && (blockColumn.children.length > 1 || !!blockColumn.textContent.trim())
+      ) {
+        wrap(blockColumn);
+      }
+    }
+  });
+}
+
 /**
  * Decorates paragraphs containing a single link as buttons.
  * @param {Element} element container element
@@ -635,6 +677,7 @@ function decorateBlock(block) {
     block.classList.add('block');
     block.dataset.blockName = shortBlockName;
     block.dataset.blockStatus = 'initialized';
+    wrapTextNodes(block);
     const blockWrapper = block.parentElement;
     blockWrapper.classList.add(`${shortBlockName}-wrapper`);
     const section = block.closest('.section');
@@ -723,4 +766,5 @@ export {
   toClassName,
   updateSectionsStatus,
   waitForLCP,
+  wrapTextNodes,
 };
diff --git a/src/block-loader.js b/src/block-loader.js
@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import { loadCSS } from './dom-utils.js';
+import { loadCSS, wrapTextNodes } from './dom-utils.js';
 
 /**
  * Updates all section status in a container element.
@@ -124,6 +124,7 @@ export function decorateBlock(block) {
     block.classList.add('block');
     block.dataset.blockName = shortBlockName;
     block.dataset.blockStatus = 'initialized';
+    wrapTextNodes(block);
     const blockWrapper = block.parentElement;
     blockWrapper.classList.add(`${shortBlockName}-wrapper`);
     const section = block.closest('.section');

diff --git a/src/dom-utils.js b/src/dom-utils.js
@@ -124,3 +124,37 @@ export function decorateTemplateAndTheme() {
   const theme = getMetadata('theme');
   if (theme) addClasses(document.body, theme);
 }
+
+/**
+ * Wrap inline text content of block cells within a <p> tag.
+ * @param {Element} block the block element
+ */
+export function wrapTextNodes(block) {
+  const validWrappers = [
+    'P',
+    'PRE',
+    'UL', 'OL',
+    'PICTURE',
+    'TABLE',
+    'H1', 'H2', 'H3', 'H4', 'H5', 'H6',
+  ];
+
+  const wrap = (el) => {
+    const wrapper = document.createElement('p');
+    wrapper.append(...el.childNodes);
+    el.append(wrapper);
+  };
+
+  block.querySelectorAll(':scope > div > div').forEach((blockColumn) => {
+    if (blockColumn.hasChildNodes()) {
+      const hasWrapper = !!blockColumn.firstElementChild
+        && validWrappers.some((tagName) => blockColumn.firstElementChild.tagName === tagName);
+      if (!hasWrapper) {
+        wrap(blockColumn);
+      } else if (blockColumn.firstElementChild.tagName === 'PICTURE'
+        && (blockColumn.children.length > 1 || !!blockColumn.textContent.trim())) {
+        wrap(blockColumn);
+      }
+    }
+  });
+}
diff --git a/src/index.js b/src/index.js
@@ -20,6 +20,7 @@ export {
   getMetadata,
   createOptimizedPicture,
   decorateTemplateAndTheme,
+  wrapTextNodes,
 } from './dom-utils.js';
 export { decorateButtons, decorateIcons, decorateSections } from './decorate.js';
 export { sampleRUM } from '@adobe/helix-rum-js';

diff --git a/test/dom-utils/wrapTextNodes.test.html b/test/dom-utils/wrapTextNodes.test.html
@@ -0,0 +1,176 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <script src="/some/path/scripts/scripts.js"></script>
+</head>
+
+<body>
+  <main>
+    <div>
+      <div class="text original">
+        <div>
+          <div>This will get a wrapping p</div>
+          <div><p>This will get a not get wrapping p since it already has one</p></div>
+          <div>  </div> <!-- I'm not sure pipeline can produce this structure, but adding it JIC -->
+        </div>
+      </div>
+      <div class="text expected">
+        <div>
+          <div><p>This will get a wrapping p</p></div>
+          <div><p>This will get a not get wrapping p since it already has one</p></div>
+          <div><p>  </p></div> <!-- I'm not sure pipeline can produce this structure, but adding it JIC -->
+        </div>
+      </div>
+      <div class="icons original">
+        <div>
+          <div><span class="icon icon-name"></span> it also works when the icon comes first</div>
+          <div><span class="icon icon-name"></span></div>
+        </div>
+      </div>
+      <div class="icons expected">
+        <div>
+          <div><p><span class="icon icon-name"></span> it also works when the icon comes first</p></div>
+          <div><p><span class="icon icon-name"></span></p></div>
+        </div>
+      </div>
+      <div class="code original">
+        <div>
+          <div><code>code blocks also get wrapped</code></div>
+          <div>text with some <code>inline code block also get wrapped</code></div>
+          <div><code>inline code block also get wrapped</code> with text after it, too</div>
+          <div>and for completeness <code>inline code block also get wrapped</code> with text before and after it!</div>
+        </div>
+        <div>
+          <div><pre><code>pre is a block level element so it doens't get a wrapper</code></pre></div>
+        </div>
+      </div>
+      <div class="code expected">
+        <div>
+          <div><p><code>code blocks also get wrapped</code></p></div>
+          <div><p>text with some <code>inline code block also get wrapped</code></p></div>
+          <div><p><code>inline code block also get wrapped</code> with text after it, too</p></div>
+          <div><p>and for completeness <code>inline code block also get wrapped</code> with text before and after it!</p></div>
+        </div>
+        <div>
+          <div><pre><code>pre is a block level element so it doens't get a wrapper</code></pre></div>
+        </div>
+      </div>
+      <div class="headings original">
+        <div>
+          <div>
+            <h2>headings will not be wrapped</h2>
+          </div>
+          <div>
+            <h3>headings of all levels will not be wrapped</h3>
+          </div>
+        </div>
+      </div>
+      <div class="headings expected">
+        <div>
+          <div>
+            <h2>headings will not be wrapped</h2>
+          </div>
+          <div>
+            <h3>headings of all levels will not be wrapped</h3>
+          </div>
+        </div>
+      </div>
+      <div class="pictures original">
+        <div>
+          <div><picture><img src="./media_123123123.png"></picture></div>
+          <div>
+            <p><picture><img src="./media_123123123.png"></picture></p>
+            <p>a picture with some text</p>
+          </div>
+          <div><picture><img src="./media_123123123.png"></picture> a picture with inline text</div>
+          <div>inline text with a picture<picture><img src="./media_123123123.png"></picture></div>
+          <div><picture><img src="./media_123123123.png"></picture> <a>a picture with an inline link</a></div>
+        </div>
+      </div>
+      <div class="pictures expected">
+        <div>
+          <div><picture><img src="./media_123123123.png"></picture></div>
+          <div>
+            <p><picture><img src="./media_123123123.png"></picture></p>
+            <p>a picture with some text</p>
+          </div>
+          <div><p><picture><img src="./media_123123123.png"></picture> a picture with inline text</p></div>
+          <div><p>inline text with a picture<picture><img src="./media_123123123.png"></picture></p></div>
+          <div><p><picture><img src="./media_123123123.png"></picture> <a>a picture with an inline link</a></p></div>
+        </div>
+      </div>
+      <div class="empty-cells original">
+        <div>
+          <div></div>
+        </div>
+      </div>
+      <div class="empty-cells expected">
+        <div>
+          <div></div>
+        </div>
+      </div>
+    </div>
+  </main>
+  <script type="module">
+    /* eslint-env mocha */
+    import { runTests } from '@web/test-runner-mocha';
+    import { expect } from '@esm-bundle/chai';
+    import { wrapTextNodes } from '../../src/dom-utils.js';
+
+    runTests(() => {
+      it('adds paragraphs to text nodes inside of blocks', () => {
+        const original = document.querySelector('.original.text');
+        const expected = document.querySelector('.expected.text');
+
+        wrapTextNodes(original);
+
+        expect(original.innerHTML).to.equal(expected.innerHTML);
+      });
+
+      it('adds paragraphs to text nodes with icons inside of blocks', () => {
+        const original = document.querySelector('.original.icons');
+        const expected = document.querySelector('.expected.icons');
+
+        wrapTextNodes(original);
+
+        expect(original.innerHTML).to.equal(expected.innerHTML);
+      });
+
+      it('wraps code inside of blocks, with pre when the code is an only-child', () => {
+        const original = document.querySelector('.original.code');
+        const expected = document.querySelector('.expected.code');
+        wrapTextNodes(original);
+
+        expect(original.innerHTML).to.equal(expected.innerHTML);
+      });
+
+      it('only wraps pictures inside of blocks when they have sibling text nodes', () => {
+        const original = document.querySelector('.original.pictures');
+        const expected = document.querySelector('.expected.pictures');
+        wrapTextNodes(original);
+
+        expect(original.innerHTML).to.equal(expected.innerHTML);
+      });
+
+      it('does not modify headings', () => {
+        const original = document.querySelector('.original.headings');
+        const expected = document.querySelector('.expected.headings');
+        wrapTextNodes(original);
+
+        expect(original.innerHTML).to.equal(expected.innerHTML);
+      });
+
+      it('does not modify empty cells', () => {
+        const original = document.querySelector('.original.empty-cells');
+        const expected = document.querySelector('.expected.empty-cells');
+        wrapTextNodes(original);
+
+        expect(original.innerHTML).to.equal(expected.innerHTML);
+      });
+    });
+  </script>
+
+</body>
+
+</html>