-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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-16-sidebar-eager #65
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,3 +7,7 @@ helix-importer-ui | |
.DS_Store | ||
*.bak | ||
.idea | ||
|
||
.vscode/ | ||
.htmlvalidate.json | ||
.htmlvalidateignore |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
/* nothing here */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/* | ||
* Fragment Block | ||
* Include content from one Helix page in another. | ||
* https://www.hlx.live/developer/block-collection/fragment | ||
*/ | ||
|
||
import { | ||
decorateMain, | ||
} from '../../scripts/scripts.js'; | ||
|
||
import { | ||
loadBlocks, | ||
} from '../../scripts/aem.js'; | ||
|
||
/** | ||
* Loads a fragment. | ||
* @param {string} path The path to the fragment | ||
* @returns {HTMLElement} The root element of the fragment | ||
*/ | ||
async function loadFragment(path) { | ||
if (path && path.startsWith('/')) { | ||
const resp = await fetch(`${path}.plain.html`); | ||
if (resp.ok) { | ||
const main = document.createElement('main'); | ||
main.innerHTML = await resp.text(); | ||
decorateMain(main); | ||
await loadBlocks(main); | ||
return main; | ||
} | ||
} | ||
return null; | ||
} | ||
|
||
export default async function decorate(block) { | ||
const link = block.querySelector('a'); | ||
const path = link ? link.getAttribute('href') : block.textContent.trim(); | ||
const fragment = await loadFragment(path); | ||
if (fragment) { | ||
const fragmentSection = fragment.querySelector(':scope .section'); | ||
if (fragmentSection) { | ||
block.closest('.section').classList.add(...fragmentSection.classList); | ||
block.closest('.fragment-wrapper').replaceWith(...fragmentSection.childNodes); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ import { | |
sampleRUM, | ||
toClassName, | ||
waitForLCP, | ||
loadBlock, | ||
} from './aem.js'; | ||
import { | ||
a, div, p, span, | ||
|
@@ -158,6 +159,27 @@ function buildArticleCopyright(main) { | |
main.append(div(buildBlock('article-copyright', { elems: [] }))); | ||
} | ||
|
||
/** | ||
* Builds an article sidebar and appends it to main in a new section. | ||
* @param main | ||
*/ | ||
function buildArticleSidebar(main) { | ||
const divs = Array.from(document.querySelectorAll('.section-metadata div div')); | ||
const targetDivs = divs.filter( | ||
(d) => d.textContent.trim().toLowerCase() === 'style' || d.textContent.trim().toLowerCase() === 'sidebar', | ||
); | ||
if (targetDivs.length !== 2) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition can unfortunately be broken. Take the following example which will say that there is already a sidebar and doesn't need to be autoblocked: <div class="section-metadata">
<div>
<div>Style</div>
<div>Test1</div>
</div>
</div>
<div class="section-metadata">
<div>
<div>Test2</div>
<div>Sidebar</div>
</div>
</div> Also the Style property can have multiple values coma separated and with spaces, e.g.: <div class="section-metadata">
<div>
<div>Style</div>
<div>Sidebar, Test1, Test2</div>
</div>
</div> https://github.com/hlxsites/servicenow/blob/main/scripts/aem.js#L453 So we need a condition that is a little bit more robust. |
||
// the article did not come with an inline sidebar | ||
const locInfo = getLocaleInfo(); | ||
const sidebarBlock = buildBlock('fragment', [ | ||
[a({ href: `${locInfo.placeholdersPrefix}/fragments/sidebar-fragment` }, 'Sidebar')], | ||
]); | ||
sidebarBlock.dataset.eagerBlock = true; | ||
const sidebar = div(sidebarBlock); // wrap sidebarBlock in div to create a new section | ||
main.append(sidebar); | ||
} | ||
} | ||
|
||
/** | ||
* Returns true if the page is an article based on the template metadata. | ||
* @returns {boolean} | ||
|
@@ -177,9 +199,13 @@ function isArticlePage() { | |
*/ | ||
// eslint-disable-next-line no-unused-vars | ||
function buildAutoBlocks(main) { | ||
if (main.parentNode !== document.body) { // don't build auto blocks in fragments | ||
return; | ||
} | ||
try { | ||
if (isArticlePage()) { | ||
buildArticleHeader(main); | ||
buildArticleSidebar(main); | ||
buildArticleCopyright(main); | ||
} | ||
buildBlogHeader(main); | ||
|
@@ -189,8 +215,14 @@ function buildAutoBlocks(main) { | |
} | ||
} | ||
|
||
function detectSidebar(main) { | ||
async function loadEagerBlocks(main) { | ||
const eagerBlocks = main.querySelectorAll('div[data-eager-block]'); | ||
await Promise.all([...eagerBlocks].map((eagerBlock) => loadBlock(eagerBlock))); | ||
} | ||
|
||
async function detectSidebar(main) { | ||
const sidebar = main.querySelector('.section.sidebar'); | ||
|
||
if (sidebar) { | ||
main.classList.add('has-sidebar'); | ||
const sidebarOffset = Number.parseInt( | ||
|
@@ -242,7 +274,6 @@ export function decorateMain(main) { | |
buildAutoBlocks(main); | ||
decorateSections(main); | ||
decorateBlocks(main); | ||
detectSidebar(main); | ||
} | ||
|
||
/** | ||
|
@@ -255,6 +286,8 @@ async function loadEager(doc) { | |
const main = doc.querySelector('main'); | ||
if (main) { | ||
decorateMain(main); | ||
await loadEagerBlocks(main); | ||
await detectSidebar(main); | ||
document.body.classList.add('appear'); | ||
await waitForLCP(LCP_BLOCKS); | ||
} | ||
|
@@ -302,7 +335,7 @@ function loadDelayed() { | |
// load anything that can be postponed to the latest here | ||
} | ||
|
||
async function loadPage() { | ||
export async function loadPage() { | ||
await loadEager(document); | ||
await loadLazy(document); | ||
loadDelayed(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to transport the
fragmentSection
's data set to the target section too, to ensure that additional section metadata configs are added, but without overriding the main's section data set (e.g.data-section-status
);I'm thinking something like: