Skip to content

Commit

Permalink
perf: improve the performance of 'Static Content Page' component (#1603)
Browse files Browse the repository at this point in the history
* improve the performance of 'Static Content Page' component renderings by optimizing/limiting the '/pagetree' REST calls regarding the used 'depth'

BREAKING CHANGES: An empty 'NavigationDepth' value of the 'Static Content Page' component now defaults to '0' instead of no depth limitation, that resulted in the whole content page tree being fetched and saved to the state (see [Migrations / From 5.0 to 5.1](https://github.com/intershop/intershop-pwa/blob/develop/docs/guides/migrations.md#from-50-to-51) for more details).
  • Loading branch information
shauke authored Mar 11, 2024
1 parent b08443c commit a6189fe
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 18 deletions.
7 changes: 7 additions & 0 deletions docs/guides/migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ If in a project other header types are used these header types and the rendering
With the introduction of the new [navigation CMS components](../concepts/cms-integration.md#navigation-cms-components) it became necessary to adapt the main navigation styling.
The styling can no longer rely on child selectors (`>`) since the CMS manged components introduce a lot of additional HTML tags around the styling relevant `li` tags.

To improve the performance of the 'Static Content Page' component the triggered `/pagetree` REST calls default behavior regarding the used `depth` was changed.
In the past no given 'Navigation Depth' in the ICM backend configuration resulted in no limitation at all ("Define how many levels the navigation tree displays.
To show all levels, leave the field empty.").
With the content model adaptions of `icm-as-customization-headless:1.7.0` a depth default value of `5` was introduced and the description was changed accordingly.
In the PWA the rendering was adapted as well so that an empty `NavigationDepth` value of the 'Static Content Page' component now defaults to `0` instead of no depth limitation, that resulted in the whole content page tree being fetched and saved to the state.
To keep the current behavior in an existing project either adapt the `0` default in `pagelet.numberParam('NavigationDepth', 0)"` to a reasonable number or set the 'Navigation Depth' values for all 'Static Content Page' component instances in the ICM backend to reasonable depth values instead of leaving them empty.

## From 4.2 to 5.0

Starting with the Intershop PWA 5.0 we develop and test against an Intershop Commerce Management 11 server.
Expand Down
3 changes: 2 additions & 1 deletion src/app/core/facades/cms.facade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ export class CMSFacade {
}

contentPageTree$(rootId: string, depth: number) {
this.store.dispatch(loadContentPageTree({ rootId, depth }));
// fetch only the depth that is actually needed, depth=0 returns already the next child level
this.store.dispatch(loadContentPageTree({ rootId, depth: depth > 0 ? depth - 1 : 0 }));
return this.store.pipe(select(getContentPageTree(rootId)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<ng-container *ngIf="contentPageTree$ | async as contentPageTree">
<ish-content-navigation
[contentPageTree]="contentPageTree"
[depth]="pagelet.numberParam('NavigationDepth')"
[depth]="pagelet.numberParam('NavigationDepth', 0)"
/>
</ng-container>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ export class CMSStaticPageComponent implements CMSComponent, OnChanges {
if (this.pagelet?.stringParam('NavigationRoot')) {
this.contentPageTree$ = this.cmsFacade.contentPageTree$(
this.pagelet.stringParam('NavigationRoot'),
this.pagelet.numberParam('NavigationDepth')
this.pagelet.numberParam('NavigationDepth', 0)
);
}

// explicitly set breadcrumb data for content pages that use the Static Content Page Component
// explicitly set breadcrumb data for content pages that use the Static Content component
// to have the complete breadcrumb data it is necessary that the content page tree
// was fetched for the given 'NavigationRoot' even if the navigation is not shown in the side panel
this.cmsFacade.setBreadcrumbForContentPage(this.pagelet?.stringParam('NavigationRoot'));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<ul>
<ng-container
[ngTemplateOutlet]="pageTreeNode"
[ngTemplateOutletContext]="{ treeNodes: [contentPageTree], counter: 0 }"
[ngTemplateOutletContext]="{ treeNodes: [contentPageTree], counter: 1 }"
/>
</ul>

Expand All @@ -13,10 +13,7 @@
[ngClass]="{ 'page-navigation-active': treeNode.contentPageId === currentContentPage.id }"
>
<a [routerLink]="treeNode | ishContentPageRoute" [title]="treeNode.name">{{ treeNode.name }}</a>
<ul
*ngIf="treeNode.children.length > 0 && ((!depth && depth !== 0) || depth >= counter)"
[ngClass]="'page-navigation-' + counter"
>
<ul *ngIf="treeNode.children.length && depth > counter" [ngClass]="'page-navigation-' + counter">
<ng-container
[ngTemplateOutlet]="pageTreeNode"
[ngTemplateOutletContext]="{ treeNodes: treeNode.children, counter: counter + 1 }"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ describe('Content Navigation Component', () => {
});

it('should split page tree, when given maxDepth is smaller than page tree depth', () => {
component.depth = 0;
component.depth = 2;
fixture.detectChanges();

expect(element.querySelectorAll('a')).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -172,7 +172,7 @@ describe('Content Navigation Component', () => {
<a ng-reflect-router-link="/page-1/page-1.a-pg1.A" title="Page 1.A" href="/page-1/page-1.a-pg1.A"
>Page 1.A</a
>
<ul ng-reflect-ng-class="page-navigation-1" class="page-navigation-1">
<ul ng-reflect-ng-class="page-navigation-2" class="page-navigation-2">
<li>
<a
ng-reflect-router-link="/page-1/page-1.a/page-1.a.a-pg"
Expand Down Expand Up @@ -201,18 +201,18 @@ describe('Content Navigation Component', () => {
fixture.detectChanges();
});

it('should set page-navigation-0 class to first layer', () => {
expect(element.querySelectorAll('.page-navigation-0')).toMatchInlineSnapshot(`
it('should set page-navigation-1 class to first layer', () => {
expect(element.querySelectorAll('.page-navigation-1')).toMatchInlineSnapshot(`
NodeList [
<ul ng-reflect-ng-class="page-navigation-0" class="page-navigation-0">
<ul ng-reflect-ng-class="page-navigation-1" class="page-navigation-1">
<li>
<a
ng-reflect-router-link="/page-1/page-1.a-pg1.A"
title="Page 1.A"
href="/page-1/page-1.a-pg1.A"
>Page 1.A</a
>
<ul ng-reflect-ng-class="page-navigation-1" class="page-navigation-1">
<ul ng-reflect-ng-class="page-navigation-2" class="page-navigation-2">
<li>
<a
ng-reflect-router-link="/page-1/page-1.a/page-1.a.a-pg"
Expand Down Expand Up @@ -244,10 +244,10 @@ describe('Content Navigation Component', () => {
`);
});

it('should set page-navigation-1 class to second layer', () => {
expect(element.querySelectorAll('.page-navigation-1')).toMatchInlineSnapshot(`
it('should set page-navigation-2 class to second layer', () => {
expect(element.querySelectorAll('.page-navigation-2')).toMatchInlineSnapshot(`
NodeList [
<ul ng-reflect-ng-class="page-navigation-1" class="page-navigation-1">
<ul ng-reflect-ng-class="page-navigation-2" class="page-navigation-2">
<li>
<a
ng-reflect-router-link="/page-1/page-1.a/page-1.a.a-pg"
Expand Down

0 comments on commit a6189fe

Please sign in to comment.