Skip to content

Commit 7c28913

Browse files
committed
breaking(pat navigation): Improve performance by removing the unnecessary mutation observer.
Since we're almost always using pat-inject for replacing or adding DOM nodes and we already have support for pat-inject here, the mutation observer is not necessary. Removing it improves the performance in situations where the navigation structure is updated - for example off-canvas navigation updates with pat-tabs would invoke many mutation observer callback hits. The previous performance improvement solved the performance penalty by deferring the callback for 10ms, but this is taking that further by avoiding it at all.
1 parent 09d533e commit 7c28913

File tree

2 files changed

+0
-65
lines changed

2 files changed

+0
-65
lines changed

src/pat/navigation/navigation.js

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import Base from "../../core/base";
22
import Parser from "../../core/parser";
33
import logging from "../../core/logging";
44
import events from "../../core/events";
5-
import utils from "../../core/utils";
65

76
const log = logging.getLogger("navigation");
87

@@ -61,21 +60,6 @@ export default Base.extend({
6160
});
6261
this.el.querySelector(`a.${current}, .${current} a`)?.click();
6362
}
64-
65-
const debounced_init_markings = utils.debounce(
66-
this.init_markings.bind(this),
67-
100
68-
);
69-
// Re-init when navigation changes.
70-
const observer = new MutationObserver(() => {
71-
debounced_init_markings();
72-
});
73-
observer.observe(this.el, {
74-
childList: true,
75-
subtree: true,
76-
attributes: false,
77-
characterData: false,
78-
});
7963
},
8064

8165
/**

src/pat/navigation/navigation.test.js

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -152,55 +152,6 @@ describe("Navigation pattern tests", function () {
152152
});
153153
});
154154

155-
describe("Navigation pattern tests - no predefined structure", function () {
156-
it("Reacts on DOM change", async function () {
157-
document.body.innerHTML = `
158-
<div id="injected_nav">
159-
<div class="w1">
160-
<a href="/path/to" class="a1">link a1</a>
161-
<div class="w11">
162-
<a href="/path/to/test" class="a11">link a11</a>
163-
</div>
164-
</div>
165-
</div>
166-
<a
167-
href="#injected_nav"
168-
class="pat-inject load-nav"
169-
data-pat-inject="target: #injection_target">load navigation</a>
170-
<nav
171-
id="injection_target"
172-
class="pat-navigation nav"
173-
data-pat-navigation="item-wrapper: div">
174-
</nav>
175-
`;
176-
177-
// TODO: change when using Jest: https://remarkablemark.org/blog/2018/11/17/mock-window-location/
178-
history.pushState(null, "", "/path/to/test");
179-
180-
Registry.scan(document.body);
181-
182-
const nav = document.querySelector("nav");
183-
const load_nav = document.querySelector(".load-nav");
184-
load_nav.click();
185-
186-
await utils.timeout(120); // wait for MutationObserver
187-
188-
const w1 = nav.querySelector(".w1");
189-
const a1 = nav.querySelector(".a1");
190-
const w11 = nav.querySelector(".w11");
191-
const a11 = nav.querySelector(".a11");
192-
193-
expect(w1.classList.contains("current")).toBeFalsy();
194-
expect(w1.classList.contains("navigation-in-path")).toBeTruthy();
195-
expect(a1.classList.contains("current")).toBeFalsy();
196-
expect(a1.classList.contains("navigation-in-path")).toBeTruthy();
197-
expect(w11.classList.contains("current")).toBeTruthy();
198-
expect(w11.classList.contains("navigation-in-path")).toBeFalsy();
199-
expect(a11.classList.contains("current")).toBeTruthy();
200-
expect(a11.classList.contains("navigation-in-path")).toBeFalsy();
201-
});
202-
});
203-
204155
describe("Navigation pattern tests - Mark items based on URL", () => {
205156
let _window_location;
206157

0 commit comments

Comments
 (0)