Skip to content

pat-inject code optimizations #1222

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

Merged
merged 9 commits into from
Jan 12, 2025
+182 −138
Merged

pat-inject code optimizations #1222

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1b19844
fix(core dom): Do not break querySelectorAllAndMe, if passed element …
thet Jan 12, 2025
3369d3a
pat-inject: Improve code for updating the history object.
thet Jan 3, 2024
5c3c1aa
pat-inject: Minor code optimization.
thet Jan 4, 2024
fa07469
pat-inject: Minor code optimization: Consistently use nodeName over t…
thet Jan 12, 2025
d5f69c2
pat-inject: Minor code optimization: Rework most forEach to for loops.
thet Jan 12, 2025
9a55aa3
pat-inject: Code modernization: Remove dependency on jquery-ext.
thet Jan 12, 2025
cdc346a
pat-inject: Minor code optimization.
thet Jan 12, 2025
b0f94fb
maint(pat inject): Modernize some parts of pat inject.
thet May 16, 2023
8a3af53
maint(pat-inject): Use create_uuid for generating the temporary autol…
thet Jan 12, 2025
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
2 changes: 1 addition & 1 deletion src/core/dom.js
View file
Original file line number Diff line number Diff line change
@@ -60,7 +60,7 @@ const toNodeArray = (nodes) => {
* @returns {Array} - The DOM nodes found.
*/
const querySelectorAllAndMe = (el, selector) => {
if (!el) {
if (!el || !el.querySelectorAll) {
return [];
}

9 changes: 9 additions & 0 deletions src/core/dom.test.js
View file
Original file line number Diff line number Diff line change
@@ -174,6 +174,15 @@ describe("core.dom tests", () => {

done();
});

it("return empty list, if the element is not a real element.", (done) => {
const res = dom.querySelectorAllAndMe("text", ".selector");
expect(Array.isArray(res)).toBe(true);
expect(res.length).toBe(0);

done();
});

});

describe("wrap tests", () => {
309 changes: 172 additions & 137 deletions src/pat/inject/inject.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import "../../core/jquery-ext"; // for findInclusive
import "../../core/polyfills"; // SubmitEvent.submitter for Safari < 15.4 and jsDOM
import $ from "jquery";
import ajax from "../ajax/ajax";
@@ -7,6 +6,7 @@ import events from "../../core/events";
import logging from "../../core/logging";
import Parser from "../../core/parser";
import registry from "../../core/registry";
import create_uuid from "../../core/uuid";
import utils from "../../core/utils";

const log = logging.getLogger("pat.inject");
@@ -38,9 +38,9 @@ parser.addArgument("class"); // Add a class to the injected content.
parser.addArgument("history", "none", ["none", "record"]);
parser.addArgument("push-marker");
parser.addArgument("scroll");
// XXX: this should not be here but the parser would bail on
// unknown parameters and expand/collapsible need to pass the url
// to us

// Note: this should not be here but the parser would bail on unknown
// parameters and expand/collapsible need to pass the url to us.
parser.addArgument("url");

const inject = {
@@ -56,12 +56,15 @@ const inject = {
// from pat-inject. Waiting a tick in pat-inject solves this -
// pat-validation's event handlers are initialized first.
await utils.timeout(1);

const el = utils.jqToNode($el);

const cfgs = this.extractConfig($el, opts);
if (cfgs.some((e) => e.history === "record") && !("pushState" in history)) {
// if the injection shall add a history entry and HTML5 pushState
// is missing, then don't initialize the injection.
log.warn("HTML5 pushState is missing, aborting");
return $el;
return;
}
$el.data("pat-inject", cfgs);

@@ -70,26 +73,27 @@ const inject = {
// exists in the page, we do not activate the injection
// but instead just change the anchors href.

// XXX: This is used in only one project for linked
// fullcalendars, it's sanity is wonky and we should
// probably solve it differently.
if ($el.is("a") && $(cfgs[0].nextHref).length > 0) {
// Note: This is used in only one project for linked fullcalendars,
// it's sanity is wonky and we should probably solve it differently.
if (el.nodeName === "A" && $(cfgs[0].nextHref).length > 0) {
log.debug(
"Skipping as next href is anchor, which already exists",
cfgs[0].nextHref
);
// XXX: reconsider how the injection enters exhausted state
return $el.attr({
href: (window.location.href.split("#")[0] || "") + cfgs[0].nextHref,
});
// TODO: reconsider how the injection enters exhausted state
el.setAttribute(
"href",
(window.location.href.split("#")[0] || "") + cfgs[0].nextHref
);
return $el;
}
}
if (cfgs[0].pushMarker) {
$("body").on("push", (event, data) => {
log.debug("received push message: " + data);
if (data == cfgs[0].pushMarker) {
log.debug("re-injecting " + data);
this.onTrigger({ currentTarget: $el[0] });
this.onTrigger({ currentTarget: el });
}
});
}
@@ -98,16 +102,16 @@ const inject = {
} else {
switch (cfgs[0].trigger) {
case "default":
cfgs.forEach((cfg) => {
for (const cfg of cfgs) {
if (cfg.delay) {
cfg.processDelay = cfg.delay;
}
});
}
// setup event handlers
if ($el[0]?.tagName === "FORM") {
log.debug("Initializing form with injection on", $el[0]);
if (el?.nodeName === "FORM") {
log.debug("Initializing form with injection on", el);
events.add_event_listener(
$el[0],
el,
"submit",
"pat-inject--form-submit",
(e) => {
@@ -123,34 +127,38 @@ const inject = {
this.onTrigger(e);
}
);
} else if ($el.is(".pat-subform")) {
} else if (el?.matches(".pat-subform")) {
log.debug("Initializing subform with injection");
} else {
$el.on("click.pat-inject", this.onTrigger.bind(this));
}
break;
case "autoload":
if (!cfgs[0].delay) {
this.onTrigger({ currentTarget: $el[0] });
this.onTrigger({ currentTarget: el });
} else {
// generate UID
const uid = Math.random().toString(36);
$el.attr("data-pat-inject-uid", uid);
// generate UUID
const uuid = create_uuid();
el.setAttribute("data-pat-inject-uuid", uuid);

// function to trigger the autoload and mark as triggered
const delayed_trigger = (uid_) => {
const delayed_trigger = (uuid_) => {
// Check if the element has been removed from the dom
const still_there = $(
"[data-pat-inject-uid='" + uid_ + "']"
const still_there = document.querySelector(
`[data-pat-inject-uuid="${uuid_}"]`
);
if (still_there.length == 0) return false;
if (!still_there) {
return false;
}

$el.data("pat-inject-autoloaded", true);
this.onTrigger({ currentTarget: $el[0] });
this.onTrigger({ currentTarget: el });
// Cleanup again.
still_there.removeAttribute("data-pat-inject-uuid");
return true;
};
window.setTimeout(
delayed_trigger.bind(null, uid),
delayed_trigger.bind(null, uuid),
cfgs[0].delay
);
}
@@ -164,7 +172,7 @@ const inject = {
}
}

log.debug("initialised:", $el);
log.debug("initialised:", el);
return $el;
},

@@ -183,10 +191,11 @@ const inject = {
// We want an AJAX request instead.
e.preventDefault && e.preventDefault();

const $el = $(e.currentTarget);
const el = e.currentTarget;
const $el = $(el);
let cfgs = $el.data("pat-inject");
if ($el[0].tagName === "FORM" && e.type === "submit") {
const form = $el[0];
if (el.nodeName === "FORM" && e.type === "submit") {
const form = el;
const submitter = e.submitter;

// Do not submit invalid forms, if validation is active.
@@ -211,6 +220,7 @@ const inject = {
cfgs = this.extractConfig($(cfg_node), opts);
}

// store the params of the form in the config, to be used by history
for (const cfg of cfgs) {
cfg.params = $.param($el.serializeArray());
}
@@ -223,13 +233,13 @@ const inject = {
submitSubform($sub) {
/* This method is called from pat-subform
*/
const $el = $sub.parents("form");
const $el = $($sub[0].closest("form"));
const cfgs = $sub.data("pat-inject");

// store the params of the subform in the config, to be used by history
$(cfgs).each((i, v) => {
v.params = $.param($sub.serializeArray());
});
for (const cfg of cfgs) {
cfg.params = $.param($sub.serializeArray());
}

try {
$el.trigger("patterns-inject-triggered");
@@ -239,19 +249,20 @@ const inject = {
this.execute(cfgs, $el);
},

extractConfig($el, opts) {
opts = $.extend({}, opts);
extractConfig($el, options = {}) {
const el = utils.jqToNode($el);
options = Object.assign({}, options); // copy

const cfgs = parser.parse($el, opts, true);
cfgs.forEach((cfg) => {
const cfgs = parser.parse($el, options, true);
for (const cfg of cfgs) {
cfg.$context = $el;
// opts and cfg have priority, fall back to href/action
// options and cfg have priority, fall back to href/action
cfg.url =
opts.url ||
options.url ||
cfg.url ||
$el.attr("href") ||
$el.attr("action") ||
$el.parents("form").attr("action") ||
el?.getAttribute("href") ||
el?.getAttribute("action") ||
el?.closest("form")?.getAttribute("action") ||
"";

// separate selector from url
@@ -275,7 +286,7 @@ const inject = {
}
}
cfg.processDelay = 0;
});
}
return cfgs;
},

@@ -339,7 +350,6 @@ const inject = {
return false;
}
cfg.$target = this.createTarget(cfg.target);
cfg.$injected = cfg.$target;
}
return true;
},
@@ -431,8 +441,8 @@ const inject = {
createTarget(selector) {
/* create a target that matches the selector
*
* XXX: so far we only support #target and create a div with
* that id appended to the body.
* Note: so far we only support #target and create a div with that id
* appended to the body.
*/
if (selector.slice(0, 1) !== "#") {
log.error("only id supported for non-existing target");
@@ -447,50 +457,66 @@ const inject = {
/* Called after the XHR has succeeded and we have a new $source
* element to inject.
*/
if (cfg.sourceMod === "content") {
$source = $source.contents();
const wrapper = document.createElement("div");
if ($source.length > 0) {
if (cfg.sourceMod === "content") {
wrapper.innerHTML = $source[0].innerHTML;
} else {
wrapper.innerHTML = $source[0].outerHTML;
}

for (const img of wrapper.querySelectorAll("img")) {
events.add_event_listener(
img,
"load",
"inject_img_load",
(e) => {
$(e.currentTarget).trigger("pat-inject-content-loaded");
},
{ once: true }
);
}
}
let $src;
$src = $source.safeClone();
$src.findInclusive("img").on("load", (e) => {
$(e.currentTarget).trigger("pat-inject-content-loaded");
});

const $injected = cfg.$injected || $src;
// Copy, because after insertion wrapper.children is empty.
const source_nodes = [...wrapper.childNodes];

// Now the injection actually happens.
if (this._inject(trigger, $src, $(target), cfg)) {
if (this._inject(trigger, source_nodes, target, cfg)) {
// Update history
this._update_history(cfg, trigger, title);
// Post-injection
this._afterInjection($el, $injected, cfg);
this._afterInjection($el, $(source_nodes), cfg);
}
},

_update_history(cfg, trigger, title) {
// History support. if subform is submitted, append form params
const glue = cfg.url.indexOf("?") > -1 ? "&" : "?";
if (cfg.history === "record" && "pushState" in history) {
if (cfg.params) {
history.pushState(
{ url: cfg.url + glue + cfg.params },
"",
cfg.url + glue + cfg.params
);
} else {
history.pushState({ url: cfg.url }, "", cfg.url);
}
// Also inject title element if we have one
if (title)
this._inject(trigger, title, $("title"), {
if (cfg.history !== "record" || !history?.pushState) {
return;
}
let url = cfg.url;
const glue = url.indexOf("?") > -1 ? "&" : "?";
if (cfg.params) {
url = `${url}${glue}${cfg.params}`;
}
history.pushState({ url: url }, "", url);
// Also inject title element if we have one
if (title) {
const title_el = document.querySelector("title");
if (title_el) {
this._inject(trigger, title, title_el, {
action: "element",
});
}
}
},

_afterInjection($el, $injected, cfg) {
/* Set a class on the injected elements and fire the
* patterns-injected event.
*/
const el = utils.jqToNode($el);
$injected
.filter((idx, el_) => {
return el_.nodeType !== TEXT_NODE;
@@ -503,14 +529,14 @@ const inject = {
// The event handler should check whether the
// injected element and the triggered element are
// the same.
$injected.parent().trigger("patterns-injected", [cfg, $el[0], $injected[0]]);
$injected.parent().trigger("patterns-injected", [cfg, el, $injected[0]]);
} else {
$injected.each((idx, el_) => {
// patterns-injected event will be triggered for each injected (non-text) element.
if (el_.nodeType !== TEXT_NODE) {
$(el_)
.addClass(cfg["class"])
.trigger("patterns-injected", [cfg, $el[0], el_]);
.trigger("patterns-injected", [cfg, el, el_]);
}
});
}
@@ -521,7 +547,7 @@ const inject = {
// 2) getting the element to scroll to (if not "top")
const scroll_target = ["top", "target"].includes(cfg.scroll)
? cfg.$target[0]
: $injected.findInclusive(cfg.scroll)[0];
: dom.querySelectorAllAndMe($injected[0], cfg.scroll);

const scroll_container = dom.find_scroll_container(
scroll_target,
@@ -536,7 +562,7 @@ const inject = {
}
}

$el[0].dispatchEvent(
el.dispatchEvent(
new Event("pat-inject-success", { bubbles: true, cancelable: true })
);
},
@@ -563,14 +589,14 @@ const inject = {
sources$ &&
sources$[sources$.length - 1] &&
sources$[sources$.length - 1][0] &&
sources$[sources$.length - 1][0].nodeName == "TITLE"
sources$[sources$.length - 1][0].nodeName === "TITLE"
) {
title = sources$[sources$.length - 1];
}
cfgs.forEach((cfg, idx1) => {
const perform_inject = () => {
if (cfg.target !== "none")
cfg.$target.each((idx2, target) => {
if (cfg.target !== "none") {
for (const target of cfg.$target) {
this._performInjection(
target,
$el,
@@ -579,7 +605,8 @@ const inject = {
ev.target,
title
);
});
}
}
};
if (cfg.processDelay) {
setTimeout(() => perform_inject(), cfg.processDelay);
@@ -646,13 +673,13 @@ const inject = {
}

// clean up
cfgs.forEach((cfg) => {
for (const cfg of cfgs) {
if ("$injected" in cfg) {
cfg.$injected.remove();
}
cfg.$target.removeClass(cfg.loadingClass);
$el.removeClass(cfg.executingClass);
});
}
$el.off("pat-ajax-success.pat-inject");
$el.off("pat-ajax-error.pat-inject");

@@ -676,6 +703,7 @@ const inject = {
* Either by making an ajax request or by spoofing an ajax
* request when the content is readily available in the current page.
*/
const el = utils.jqToNode($el);
// get a kinda deep copy, we scribble on it
cfgs = cfgs.map((cfg) => $.extend({}, cfg));
if (!this.verifyConfig(cfgs)) {
@@ -693,7 +721,7 @@ const inject = {
for (const cfg of cfgs) {
// Add a execute class on the pat-inject element.
if (cfg?.executingClass) {
$el[0].classList.add(cfg.executingClass);
el.classList.add(cfg.executingClass);
}
// Add a loading class to the target.
// Can be used for loading-spinners.
@@ -709,12 +737,12 @@ const inject = {
// is called before this one, even for non-async local injects.
await utils.timeout(1);
// Remove the close-panel event listener.
events.remove_event_listener($el[0], "pat-inject--close-panel");
events.remove_event_listener(el, "pat-inject--close-panel");
// Only close the panel if a close-panel event was catched previously.
if (do_close_panel) {
do_close_panel = false;
// Re-trigger close-panel event if it was caught while injection was in progress.
$el[0].dispatchEvent(
el.dispatchEvent(
new Event("close-panel", { bubbles: true, cancelable: true })
);
}
@@ -726,16 +754,11 @@ const inject = {

// Prevent closing the panel while injection is in progress.
let do_close_panel = false;
events.add_event_listener(
$el[0],
"close-panel",
"pat-inject--close-panel",
(e) => {
e.stopPropagation();
e.stopImmediatePropagation();
do_close_panel = true;
}
);
events.add_event_listener(el, "close-panel", "pat-inject--close-panel", (e) => {
e.stopPropagation();
e.stopImmediatePropagation();
do_close_panel = true;
});

if (cfgs[0].url.length) {
ajax.request($el, {
@@ -756,45 +779,45 @@ const inject = {
}
},

_inject(trigger, $source, $target, cfg) {
// action to jquery method mapping, except for "content"
// and "element"
const method = {
contentbefore: "prepend",
contentafter: "append",
elementbefore: "before",
elementafter: "after",
}[cfg.action];

_inject(trigger, source, target, cfg) {
if (cfg.source === "none") {
$target.replaceWith("");
// Special case. Clear the target after ajax call.
target.replaceWith("");
return true;
}
if ($source.length === 0) {
log.warn("Aborting injection, source not found:", $source);
if (source.length === 0) {
log.warn("Aborting injection, source not found:", source);
$(trigger).trigger("pat-inject-missingSource", {
url: cfg.url,
selector: cfg.source,
});
return false;
}
if (cfg.target === "none")
if (cfg.target === "none") {
// Special case. Don't do anything, we don't want any result
return true;
if ($target.length === 0) {
log.warn("Aborting injection, target not found:", $target);
}
if (!target) {
log.warn("Aborting injection, target not found:", target);
$(trigger).trigger("pat-inject-missingTarget", {
selector: cfg.target,
});
return false;
}
if (cfg.action === "content") {
$target.empty().append($source);
} else if (cfg.action === "element") {
$target.replaceWith($source);
} else {
$target[method]($source);
}

// cfg.action to DOM method mapping
const method = {
content: "replaceChildren",
contentafter: "append",
contentbefore: "prepend",
element: "replaceWith",
elementafter: "after",
elementbefore: "before",
}[cfg.action];

// Inject the content HERE!
target[method](...source);

return true;
},

@@ -878,7 +901,7 @@ const inject = {
.map(([tag, attr]) => `${tag}[${attr}]`)
.join(", ");
for (const el_ of page.querySelectorAll(rebase_selector)) {
const attr = this._rebaseAttrs[el_.tagName.toLowerCase()];
const attr = this._rebaseAttrs[el_.nodeName.toLowerCase()];
let value = el_.getAttribute(attr);

if (
@@ -966,7 +989,7 @@ const inject = {
return false;
}

const el = $el[0];
const el = utils.jqToNode($el);

// delay: default is 200ms to allow scrolling over and past autoload-visible elements without loading them.
const delay = cfgs[0].delay || 200;
@@ -1013,18 +1036,20 @@ const inject = {
},

_initIdleTrigger($el, delay) {
// XXX TODO: handle item removed from DOM
// TODO: handle item removed from DOM
const timeout = parseInt(delay, 10);
let timer;

const el = utils.jqToNode($el);

const onTimeout = () => {
this.onTrigger({ currentTarget: $el[0] });
this.onTrigger({ currentTarget: el });
unsub();
clearTimeout(timer);
};

const onInteraction = utils.debounce(() => {
if (!document.body.contains($el[0])) {
if (!document.body.contains(el)) {
unsub();
return;
}
@@ -1033,28 +1058,38 @@ const inject = {
}, timeout);

const unsub = () => {
["scroll", "resize"].forEach((e) =>
window.removeEventListener(e, onInteraction)
);
[
for (const event of ["scroll", "resize"]) {
window.removeEventListener(event, onInteraction);
}
for (const event of [
"click",
"keypress",
"keyup",
"mousemove",
"touchstart",
"touchend",
].forEach((e) => document.removeEventListener(e, onInteraction));
]) {
document.removeEventListener(event, onInteraction);
}
};

onInteraction();

["scroll", "resize"].forEach((e) => window.addEventListener(e, onInteraction));
["click", "keypress", "keyup", "mousemove", "touchstart", "touchend"].forEach(
(e) => document.addEventListener(e, onInteraction)
);
for (const event of ["scroll", "resize"]) {
window.addEventListener(event, onInteraction);
}
for (const event of [
"click",
"keypress",
"keyup",
"mousemove",
"touchstart",
"touchend",
]) {
document.addEventListener(event, onInteraction);
}
},

// XXX: simple so far to see what the team thinks of the idea
registerTypeHandler(type, handler) {
this.handlers[type] = handler;
},
@@ -1085,7 +1120,7 @@ $(document).on("patterns-injected.inject", async (ev, cfg, trigger, injected) =>
* Remove the "loading-class" classes from all injection targets and
* then scan the injected content for new patterns.
*/
if (cfg && cfg.skipPatInjectHandler) {
if (cfg?.skipPatInjectHandler) {
// Allow skipping this handler but still have other handlers in other
// patterns listen to ``patterns-injected``.
return;