henningn commented on code in PR #6099: URL: https://github.com/apache/myfaces-tobago/pull/6099#discussion_r1979374575
########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; Review Comment: expanded wird nicht verwendet -> entfernen ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; Review Comment: bootstrap wird nicht verwendet und kann entfernt werden ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; Review Comment: element wird nicht verwendet -> entfernen ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); + this.resizeHandler = this.throttle(this.adjustFixedPosition.bind(this), 100); + } + + connectedCallback(): void { + // Cache all section elements first to avoid repeated DOM queries + this.sectionElements = Array.from(document.querySelectorAll<HTMLElement>("tobago-section[id^='page:mainForm:']")); + + // Build the section tree hierarchy once + this.buildSectionTree(); + + // Render the initial tree + this.renderContentTree(); + + // Add event listeners for page changes + window.addEventListener("hashchange", this.hashChangeHandler); + window.addEventListener("resize", this.resizeHandler); + + // Use ResizeObserver instead of window resize for better performance + this.setupResizeObserver(); + + // Initial position adjustment + this.adjustFixedPosition(); + + // Initial scroll to hash if present with proper timing + if (window.location.hash) { + requestAnimationFrame(() => { Review Comment: Ich denke im connectedCallback() braucht man keinen requestAnimationFrame() ? ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); + this.resizeHandler = this.throttle(this.adjustFixedPosition.bind(this), 100); + } + + connectedCallback(): void { + // Cache all section elements first to avoid repeated DOM queries + this.sectionElements = Array.from(document.querySelectorAll<HTMLElement>("tobago-section[id^='page:mainForm:']")); + + // Build the section tree hierarchy once + this.buildSectionTree(); + + // Render the initial tree + this.renderContentTree(); + + // Add event listeners for page changes + window.addEventListener("hashchange", this.hashChangeHandler); Review Comment: "hashchange" Event habe ich nirgendwo gesehen. Kann das weg? ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); + this.resizeHandler = this.throttle(this.adjustFixedPosition.bind(this), 100); + } + + connectedCallback(): void { + // Cache all section elements first to avoid repeated DOM queries + this.sectionElements = Array.from(document.querySelectorAll<HTMLElement>("tobago-section[id^='page:mainForm:']")); + + // Build the section tree hierarchy once + this.buildSectionTree(); + + // Render the initial tree + this.renderContentTree(); + + // Add event listeners for page changes + window.addEventListener("hashchange", this.hashChangeHandler); + window.addEventListener("resize", this.resizeHandler); + + // Use ResizeObserver instead of window resize for better performance + this.setupResizeObserver(); + + // Initial position adjustment + this.adjustFixedPosition(); + + // Initial scroll to hash if present with proper timing + if (window.location.hash) { + requestAnimationFrame(() => { + this.handleHashChange(); + this.updateActiveSection(); + }); + } else { + this.updateActiveSection(); Review Comment: this.updateActiveSection() scheint in jedem Fall ausgeführt werden zu müssen. Dann kann man sich den "else"-Teil sparen und "this.updateActiveSection()" ans Ende des connectedCallback() stellen. (Spart zwei Zeilen...) ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); + this.resizeHandler = this.throttle(this.adjustFixedPosition.bind(this), 100); Review Comment: resizeHandler kann man direkt in der globalen Variable setzen. Der Konstruktor kann dann entfernt werden. ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); + this.resizeHandler = this.throttle(this.adjustFixedPosition.bind(this), 100); + } + + connectedCallback(): void { + // Cache all section elements first to avoid repeated DOM queries + this.sectionElements = Array.from(document.querySelectorAll<HTMLElement>("tobago-section[id^='page:mainForm:']")); + + // Build the section tree hierarchy once + this.buildSectionTree(); + + // Render the initial tree + this.renderContentTree(); + + // Add event listeners for page changes + window.addEventListener("hashchange", this.hashChangeHandler); + window.addEventListener("resize", this.resizeHandler); + + // Use ResizeObserver instead of window resize for better performance + this.setupResizeObserver(); + + // Initial position adjustment + this.adjustFixedPosition(); + + // Initial scroll to hash if present with proper timing + if (window.location.hash) { + requestAnimationFrame(() => { + this.handleHashChange(); + this.updateActiveSection(); + }); + } else { + this.updateActiveSection(); + } + } + + disconnectedCallback(): void { + // Clean up event listeners + window.removeEventListener("hashchange", this.hashChangeHandler); + window.removeEventListener("resize", this.resizeHandler); + + // Clean up resize observer + if (this.resizeObserver) { + this.resizeObserver.disconnect(); + this.resizeObserver = null; + } + + // Clear any pending timeouts + if (this.scrollThrottleTimeout !== null) { + window.clearTimeout(this.scrollThrottleTimeout); + this.scrollThrottleTimeout = null; + } + } + + /** + * Throttle function to limit execution frequency + */ + private throttle(func: Function, delay: number): () => void { + let lastCall = 0; + return function(): void { + const now = new Date().getTime(); + if (now - lastCall >= delay) { + lastCall = now; + func(); + } + }; + } + + /** + * Set up resize observer for more efficient layout adjustments + */ + private setupResizeObserver(): void { + if ("ResizeObserver" in window) { + this.resizeObserver = new ResizeObserver(this.throttle(() => { + this.adjustFixedPosition(); + }, 100)); + + // Observe header elements that might affect positioning + const header = document.querySelector("header"); + const navbar = document.querySelector(".navbar"); + + if (header) this.resizeObserver.observe(header); + if (navbar) this.resizeObserver.observe(navbar); + } + } + + /** + * Adjusts the fixed position based on current header height + * Using requestAnimationFrame to batch layout reads/writes + */ + private adjustFixedPosition(): void { + requestAnimationFrame(() => { + try { + // Batch all DOM reads first + const headerHeight = document.querySelector("header")?.clientHeight || 0; + const navbarHeight = document.querySelector(".navbar")?.clientHeight || 0; + const fixedTopHeight = document.querySelector(".fixed-top")?.clientHeight || 0; Review Comment: Warum so viele Variablen für den Header? Der gesuchte Header is "tobago-header.sticky-top". Für Header/Footer kann man gut einen Getter anlegen. Außerdem: Ich finde offsetHeight besser als clientHeight, weil offsetHeight auch die Border berücksichtigt. ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { Review Comment: Setter und Getter hätte ich gerne am Ende der Klasse. (Haben wir uns leider im Theme auch nicht überall dran gehalten.) ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); + this.resizeHandler = this.throttle(this.adjustFixedPosition.bind(this), 100); + } + + connectedCallback(): void { + // Cache all section elements first to avoid repeated DOM queries + this.sectionElements = Array.from(document.querySelectorAll<HTMLElement>("tobago-section[id^='page:mainForm:']")); + + // Build the section tree hierarchy once + this.buildSectionTree(); + + // Render the initial tree + this.renderContentTree(); + + // Add event listeners for page changes + window.addEventListener("hashchange", this.hashChangeHandler); + window.addEventListener("resize", this.resizeHandler); + + // Use ResizeObserver instead of window resize for better performance + this.setupResizeObserver(); + + // Initial position adjustment + this.adjustFixedPosition(); + + // Initial scroll to hash if present with proper timing + if (window.location.hash) { + requestAnimationFrame(() => { + this.handleHashChange(); + this.updateActiveSection(); + }); + } else { + this.updateActiveSection(); + } + } + + disconnectedCallback(): void { + // Clean up event listeners + window.removeEventListener("hashchange", this.hashChangeHandler); + window.removeEventListener("resize", this.resizeHandler); + + // Clean up resize observer + if (this.resizeObserver) { + this.resizeObserver.disconnect(); + this.resizeObserver = null; + } + + // Clear any pending timeouts + if (this.scrollThrottleTimeout !== null) { + window.clearTimeout(this.scrollThrottleTimeout); + this.scrollThrottleTimeout = null; + } + } + + /** + * Throttle function to limit execution frequency + */ + private throttle(func: Function, delay: number): () => void { + let lastCall = 0; + return function(): void { + const now = new Date().getTime(); + if (now - lastCall >= delay) { + lastCall = now; + func(); + } + }; + } + + /** + * Set up resize observer for more efficient layout adjustments + */ + private setupResizeObserver(): void { + if ("ResizeObserver" in window) { + this.resizeObserver = new ResizeObserver(this.throttle(() => { + this.adjustFixedPosition(); + }, 100)); + + // Observe header elements that might affect positioning + const header = document.querySelector("header"); + const navbar = document.querySelector(".navbar"); + + if (header) this.resizeObserver.observe(header); + if (navbar) this.resizeObserver.observe(navbar); + } + } + + /** + * Adjusts the fixed position based on current header height + * Using requestAnimationFrame to batch layout reads/writes + */ + private adjustFixedPosition(): void { + requestAnimationFrame(() => { + try { + // Batch all DOM reads first + const headerHeight = document.querySelector("header")?.clientHeight || 0; + const navbarHeight = document.querySelector(".navbar")?.clientHeight || 0; + const fixedTopHeight = document.querySelector(".fixed-top")?.clientHeight || 0; + + // Calculate the offset only once + const topOffset = Math.max(20, headerHeight, navbarHeight + 10, fixedTopHeight + 10); + + // Then batch all DOM writes + this.style.top = `${topOffset}px`; + this.style.maxHeight = `calc(100vh - ${topOffset + 20}px)`; Review Comment: Bei der Berechnung der Höhe muss auch der Footer beachtet werden. ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); + this.resizeHandler = this.throttle(this.adjustFixedPosition.bind(this), 100); + } + + connectedCallback(): void { + // Cache all section elements first to avoid repeated DOM queries + this.sectionElements = Array.from(document.querySelectorAll<HTMLElement>("tobago-section[id^='page:mainForm:']")); + + // Build the section tree hierarchy once + this.buildSectionTree(); + + // Render the initial tree + this.renderContentTree(); + + // Add event listeners for page changes + window.addEventListener("hashchange", this.hashChangeHandler); + window.addEventListener("resize", this.resizeHandler); + + // Use ResizeObserver instead of window resize for better performance + this.setupResizeObserver(); + + // Initial position adjustment + this.adjustFixedPosition(); + + // Initial scroll to hash if present with proper timing + if (window.location.hash) { + requestAnimationFrame(() => { + this.handleHashChange(); + this.updateActiveSection(); + }); + } else { + this.updateActiveSection(); + } + } + + disconnectedCallback(): void { + // Clean up event listeners + window.removeEventListener("hashchange", this.hashChangeHandler); + window.removeEventListener("resize", this.resizeHandler); + + // Clean up resize observer + if (this.resizeObserver) { + this.resizeObserver.disconnect(); + this.resizeObserver = null; + } + + // Clear any pending timeouts + if (this.scrollThrottleTimeout !== null) { + window.clearTimeout(this.scrollThrottleTimeout); + this.scrollThrottleTimeout = null; + } + } + + /** + * Throttle function to limit execution frequency + */ + private throttle(func: Function, delay: number): () => void { + let lastCall = 0; + return function(): void { + const now = new Date().getTime(); + if (now - lastCall >= delay) { + lastCall = now; + func(); + } + }; + } + + /** + * Set up resize observer for more efficient layout adjustments + */ + private setupResizeObserver(): void { + if ("ResizeObserver" in window) { + this.resizeObserver = new ResizeObserver(this.throttle(() => { + this.adjustFixedPosition(); + }, 100)); + + // Observe header elements that might affect positioning + const header = document.querySelector("header"); + const navbar = document.querySelector(".navbar"); + + if (header) this.resizeObserver.observe(header); + if (navbar) this.resizeObserver.observe(navbar); + } + } + + /** + * Adjusts the fixed position based on current header height + * Using requestAnimationFrame to batch layout reads/writes + */ + private adjustFixedPosition(): void { + requestAnimationFrame(() => { + try { + // Batch all DOM reads first + const headerHeight = document.querySelector("header")?.clientHeight || 0; + const navbarHeight = document.querySelector(".navbar")?.clientHeight || 0; + const fixedTopHeight = document.querySelector(".fixed-top")?.clientHeight || 0; + + // Calculate the offset only once + const topOffset = Math.max(20, headerHeight, navbarHeight + 10, fixedTopHeight + 10); + + // Then batch all DOM writes + this.style.top = `${topOffset}px`; + this.style.maxHeight = `calc(100vh - ${topOffset + 20}px)`; + } catch (error) { + console.warn("Error adjusting sidebar position:", error); + } + }); + } + + /** + * Builds the hierarchical tree structure from the DOM using a single-pass algorithm + */ + buildSectionTree(): void { + // Reset the tree + this.sectionTree = []; + + if (this.sectionElements.length === 0) { + return; + } + + // Sort by DOM order (natural document position) + // This avoids the expensive getElementPosition method + this.sectionElements.sort((a, b) => { + // Use compareDocumentPosition for efficient DOM ordering + const position = a.compareDocumentPosition(b); + return position & Node.DOCUMENT_POSITION_FOLLOWING ? -1 : 1; + }); + + // Use a stack-based approach for building the tree (single pass) + const stack: SectionNode[] = []; + + this.sectionElements.forEach(section => { + const level = this.getSectionLevel(section); + const newNode: SectionNode = { + element: section, + id: section.id, + title: this.getSectionTitle(section), + level: level, + children: [], + expanded: true + }; + + // Pop the stack until we find the parent level + while (stack.length > 0 && stack[stack.length - 1].level >= level) { + stack.pop(); + } + + if (stack.length === 0) { + // This is a top-level node + this.sectionTree.push(newNode); + } else { + // Add as child to the current parent + stack[stack.length - 1].children.push(newNode); + } + + // Push this node to the stack + stack.push(newNode); + }); + } + + /** + * Determines the section level based on nesting or heading level + * Caches section heading information to avoid repeated DOM traversal + */ + getSectionLevel(section: HTMLElement): number { + // Cache for section levels + if ((section as any)._cachedLevel) { + return (section as any)._cachedLevel; + } + + // Try to determine level from the parent-child relationship + let parent = section.parentElement; + let level = 1; + + while (parent) { + if (parent.tagName.toLowerCase() === "tobago-section") { + level++; + } + parent = parent.parentElement; + } + + // Fallback to heading level if available + if (level === 1) { + const heading = section.querySelector("h1, h2, h3, h4, h5, h6"); + if (heading) { + const headingLevel = parseInt(heading.tagName.substring(1)); + level = headingLevel - 1; + } + } + + // Cache the result + (section as any)._cachedLevel = level; + return level; + } + + /** + * Updates the active section based on current URL hash + * Uses more efficient selectors and DOM operations + */ + updateActiveSection(): void { + requestAnimationFrame(() => { + try { + // Remove active class from all links + const links = this.querySelectorAll(".sidebar-link.active"); + links.forEach(link => link.classList.remove("active")); + + // Get current hash without the # + const currentHash = window.location.hash.substring(1); + + if (currentHash) { + // Find and activate the correct link - use attribute selector for better performance + const activeLink = this.querySelector(`a.sidebar-link[href="#${CSS.escape(currentHash)}"]`); + if (activeLink) { + activeLink.classList.add("active"); + + // Expand parent sections if needed + let parent = activeLink.closest("li")?.parentElement; + while (parent) { + if (parent.classList.contains("sidebar-submenu")) { + parent.classList.add("show"); + const toggleButton = parent.previousElementSibling?.querySelector(".sidebar-toggle"); + if (toggleButton) { + toggleButton.setAttribute("aria-expanded", "true"); + toggleButton.classList.remove("collapsed"); + } + } + parent = parent.parentElement; + } + } + } + } catch (error) { + console.warn("Error updating active section:", error); + } + }); + } + + /** + * Gets the title from a section with caching for performance + */ + private getSectionTitle(section: HTMLElement): string { Review Comment: Ich glaube der _cachedTitle wird nie genutzt? Jedenfalls scheinen mir die Fallback-Lösung ein wenig komplex. ``` private getSectionTitle(section: Element): string { const titleSpan = section.querySelector(".tobago-header span"); return titleSpan ? titleSpan.textContent.trim() : section.id; } ``` Funktioniert genauso und ist viel übersichtlicher. ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); Review Comment: hashChangeHandler wird nirgendwo verwendet? Der kann wohl weg? ########## tobago-example/tobago-example-demo/src/main/scss/demo-sidebar.scss: ########## @@ -0,0 +1,92 @@ +// Variables +$sidebar-text-color: rgba(0, 0, 0, 0.65); +$sidebar-text-hover: rgba(0, 0, 0, 0.85); +$sidebar-active-color: #0d6efd; +$sidebar-hover-bg: rgba(0, 0, 0, 0.05); +$scrollbar-thumb-color: rgba(0, 0, 0, 0.2); +$scrollbar-track-color: rgba(0, 0, 0, 0.05); +$sidebar-border-color: #dee2e6; + +// Force the sidebar to be fixed +demo-sidebar { + display: block; + position: fixed; + top: 100px !important; Review Comment: dieses "top" überschreibt das berechnete "top" aus dem TypeScript. Welches soll denn gelten? ########## tobago-example/tobago-example-demo/src/main/ts/demo-sidebar.ts: ########## @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as bootstrap from "bootstrap"; +import { html, render } from "lit-html"; + +interface SectionNode { + element: HTMLElement; + id: string; + title: string; + level: number; + children: SectionNode[]; + expanded: boolean; +} + +export class Sidebar extends HTMLElement { + private sectionTree: SectionNode[] = []; + private sectionElements: HTMLElement[] = []; + private resizeObserver: ResizeObserver | null = null; + private scrollThrottleTimeout: number | null = null; + private hashChangeHandler: () => void; + private resizeHandler: () => void; + + /** + * Gets the scroll offset from an attribute or uses the default + */ + get scrollOffset(): number { + const attributeValue = this.getAttribute("scroll-offset"); + return attributeValue ? parseInt(attributeValue, 10) : 70; + } + + constructor() { + super(); + // Bind methods to preserve this context + this.hashChangeHandler = this.handleHashChange.bind(this); + this.resizeHandler = this.throttle(this.adjustFixedPosition.bind(this), 100); + } + + connectedCallback(): void { + // Cache all section elements first to avoid repeated DOM queries + this.sectionElements = Array.from(document.querySelectorAll<HTMLElement>("tobago-section[id^='page:mainForm:']")); + + // Build the section tree hierarchy once + this.buildSectionTree(); + + // Render the initial tree + this.renderContentTree(); + + // Add event listeners for page changes + window.addEventListener("hashchange", this.hashChangeHandler); + window.addEventListener("resize", this.resizeHandler); + + // Use ResizeObserver instead of window resize for better performance + this.setupResizeObserver(); + + // Initial position adjustment + this.adjustFixedPosition(); + + // Initial scroll to hash if present with proper timing + if (window.location.hash) { + requestAnimationFrame(() => { + this.handleHashChange(); + this.updateActiveSection(); + }); + } else { + this.updateActiveSection(); + } + } + + disconnectedCallback(): void { + // Clean up event listeners + window.removeEventListener("hashchange", this.hashChangeHandler); + window.removeEventListener("resize", this.resizeHandler); + + // Clean up resize observer + if (this.resizeObserver) { + this.resizeObserver.disconnect(); + this.resizeObserver = null; + } + + // Clear any pending timeouts + if (this.scrollThrottleTimeout !== null) { + window.clearTimeout(this.scrollThrottleTimeout); + this.scrollThrottleTimeout = null; + } + } + + /** + * Throttle function to limit execution frequency + */ + private throttle(func: Function, delay: number): () => void { + let lastCall = 0; + return function(): void { + const now = new Date().getTime(); + if (now - lastCall >= delay) { + lastCall = now; + func(); + } + }; + } + + /** + * Set up resize observer for more efficient layout adjustments + */ + private setupResizeObserver(): void { + if ("ResizeObserver" in window) { + this.resizeObserver = new ResizeObserver(this.throttle(() => { + this.adjustFixedPosition(); + }, 100)); + + // Observe header elements that might affect positioning + const header = document.querySelector("header"); + const navbar = document.querySelector(".navbar"); + + if (header) this.resizeObserver.observe(header); + if (navbar) this.resizeObserver.observe(navbar); + } + } + + /** + * Adjusts the fixed position based on current header height + * Using requestAnimationFrame to batch layout reads/writes + */ + private adjustFixedPosition(): void { + requestAnimationFrame(() => { + try { + // Batch all DOM reads first + const headerHeight = document.querySelector("header")?.clientHeight || 0; + const navbarHeight = document.querySelector(".navbar")?.clientHeight || 0; + const fixedTopHeight = document.querySelector(".fixed-top")?.clientHeight || 0; + + // Calculate the offset only once + const topOffset = Math.max(20, headerHeight, navbarHeight + 10, fixedTopHeight + 10); + + // Then batch all DOM writes + this.style.top = `${topOffset}px`; + this.style.maxHeight = `calc(100vh - ${topOffset + 20}px)`; + } catch (error) { + console.warn("Error adjusting sidebar position:", error); + } + }); + } + + /** + * Builds the hierarchical tree structure from the DOM using a single-pass algorithm + */ + buildSectionTree(): void { Review Comment: Das aufbauen des Trees sollte rekursiv erfolgen. Beim Rendern wird ja auch Rekursion genutzt. Methode könnte dann so aussehen: ``` private buildSectionTree(element: Element, level: number, parent: SectionNode): void { for (let i = 0; i < element.children.length; i++) { const child = element.children[i]; if (child.tagName === "TOBAGO-SECTION") { const sectionNode: SectionNode = { id: child.id, title: this.getSectionTitle(child), level: level, children: [] }; parent.children.push(sectionNode); this.buildSectionTree(child, (level + 1), sectionNode); } else { this.buildSectionTree(child, level, parent); } } } ``` und wäre dann viel kürzer. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@myfaces.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
