Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: rewrite markup out of pipeline to ensure all block div's contain appropriate paragraph tags #45

Merged
merged 2 commits into from
Mar 12, 2024
Merged

feature: rewrite markup out of pipeline to ensure all block div's contain appropriate paragraph tags #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
960366d
feature: rewrite markup during block decoration pipeline to ensure al…
shsteimer Feb 21, 2024
c0bc734
remove unused tag parameter in wrap function
shsteimer Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions dist/aem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -723,4 +766,5 @@ export {
toClassName,
updateSectionsStatus,
waitForLCP,
wrapTextNodes,
};
3 changes: 2 additions & 1 deletion src/block-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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');
Expand Down
34 changes: 34 additions & 0 deletions src/dom-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
});
}
1 change: 1 addition & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
176 changes: 176 additions & 0 deletions test/dom-utils/wrapTextNodes.test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>