I'm working in a Tableau driven system, with large numbers of charts, I needed a way to reduce the strain of the initial load of our app. It's fairly simple, but wanted to ensure I accounted for every scenario, I don't generally utilize lazy loading.
Print isn't a priority.
jsfiddle: Demo (Tableau doesn't work within SO's snippets because of the iframes)
Relevant Javascript
const throttle = (fn, delay) => {
let canCall = true;
return (...args) => {
if (canCall) {
fn.apply(null, args);
canCall = false;
setTimeout(() => {
canCall = true;
}, delay);
}
};
};
const setAttributes = (el, options) =>
Object.keys(options).forEach(attr => el.setAttribute(attr, options[attr]));
const w = window;
const d = document;
const b = d.body;
const x = w.innerWidth || e.clientWidth || b.clientWidth;
const y = w.innerHeight || e.clientHeight || b.clientHeight;
/* unecessary, but to have same object format, e.g. readability */
const win = Object.create(null);
win.y = y;
const frame = d.querySelectorAll("iframe");
/** may be over doing it here... test results */
const preloadPath = () =>
frame.forEach(item => {
const source = item.getAttribute("data-src");
const preloadLink = d.createElement("link");
const head = d.getElementsByTagName("head")[0];
setAttributes(preloadLink, {
rel: "preload",
as: "document",
type: "text/html",
crossorigin: "anonymous",
href: source
});
head.insertBefore(preloadLink, head.firstChild);
});
const loadFrame = () =>
frame.forEach(item => {
/* only run if not src is present */
if (!item.src) {
const source = item.getAttribute("data-src"); // path
const parent = item.parentElement; // wrapper
const frameRect = parent.getBoundingClientRect(); // wrapper dimensions
if (
w.pageYOffset + win.y >= frameRect.y && // if frame top exceeds page bottom
frameRect.y + frameRect.height >= w.pageYOffset // if frame bottom exceeds page top
) {
item.src = source; // set path
item.removeAttribute("data-src"); // remove placeholder attr
}
}
});
/** load on docReady */
d.addEventListener("DOMContentLoaded", [preloadPath, loadFrame]);
/** throttle on scroll */
const t = throttle(loadFrame, 64);
w.addEventListener("scroll", t);
2 Answers 2
General Feedback
The code in the jsFiddle Demo appears to function well (although it isn't the same as the code above- see the next section for an explanation). There is good usage of const
for values that are not re-assigned and functional programming with arrow functions.
Flaw with event listener setup
It appears that the fiddle has different code than appears here, but nonetheless, the code above contains the following line:
d.addEventListener("DOMContentLoaded", [preloadPath, loadFrame]);
The second argument appears to be an array. I haven't seen an array there used before and it doesn't appear to work. The Parameters section of the MDN documentation for addEventListener reads:
listener
The object which receives a notification (an object that implements theEvent
interface) when an event of the specified type occurs. This must be an object implementing theEventListener
interface, or a JavaScriptfunction
. See The event listener callback for details on the callback itself.1
If you did need to have both functions run when that event occurs, you would either need to have a single callback function that calls both or call addEventListener
once for each function. It appears that in the fiddle preloadPath
has been removed...
Suggestions
Variable naming
The variable name frame
sounds singular, yet it returns a NodeList
which would typically contain multiple DOM elements.
const frame = d.querySelectorAll("iframe");
Thus a more appropriate name would be frames
. That way when statements like frames.forEach()
is read, it implies that a function is invoked for each of the frames.
The setAttributes()
function
Correct me if I am wrong but this function appears to do the same thing that Object.assign()
. I was able to replace calls to that function with calls to Object.assign()
and still saw the attributes set as expected in Opera and Chrome.
Arrow functions with empty argument lists
You don't have to do this, but _
could be used instead of empty parentheses for arrow functions with no named arguments. Refer to this SO post and its answers for more context.
timeout in function returned by throttle()
While it would only save a couple lines, the arrow function passed to setTimeout()
in the function returned by throttle()
could be simplified to remove the curly braces. While this would mean that true
would be returned, it doesn't affect anything.
const throttle = (fn, delay) => {
let canCall = true;
return (...args) => {
if (canCall) {
fn.apply(null, args);
canCall = false;
setTimeout(_ => canCall = true, delay);
}
};
};
1https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Parameters)
-
\$\begingroup\$ yeah actually in the version I'm using it has a
const readyInit = (() => {preloadPath();loadFrame();});
function I call on DOMReady. This was actually due to me showing a collage "I wish we could" and must have overlooked it when posting a copy+paste; good eye. I was not aware of the Lambda expression for empty ()! All valid and good points, thanks for your input! I'll update the OP to reflect changes when I get a moment. \$\endgroup\$darcher– darcher2018年12月21日 17:12:17 +00:00Commented Dec 21, 2018 at 17:12 -
\$\begingroup\$ Cool; Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. You could post a follow up question with update code... \$\endgroup\$2018年12月21日 18:55:28 +00:00Commented Dec 21, 2018 at 18:55
I've updated the code based on the suggestions in comments and answers.
shout out to Sᴀᴍ Onᴇᴌᴀ for his suggestions in the accepted answer and David Knipe for suggestions on the throttle
function.
const w = window;
const d = document;
let timeout;
const throttle = (fn, ...args) => (
timeout && w.cancelAnimationFrame(timeout),
timeout = w.requestAnimationFrame(_ => fn(...args)));
const loadFrame = _ =>
d.querySelectorAll("iframe").forEach(frame => {
if (frame.src) return
const frameRect = frame.parentElement.getBoundingClientRect();
(w.pageYOffset + d.documentElement.clientHeight >= frameRect.y
&& frameRect.y + frameRect.height >= w.pageYOffset)
&& frame.setAttribute('src', frame.getAttribute("data-src"))
.removeAttribute("data-src")});
d.addEventListener("DOMContentLoaded", loadFrame, false);
w.addEventListener("scroll", _ => throttle(loadFrame), false);
Explore related questions
See similar questions with these tags.
preloadPath
? \$\endgroup\$<link>
element is used to preload a resource, thenload
event of<link>
should be used beforeloadFrame()
is called. PresentlypreloadPath
does not have direct relevance to the<iframe>
elements, though can be utilized ifimport
is used; see Is there a way to know if a link/script is still pending or has it failed; How to querySelector an element in an html import from a document that imports it?. \$\endgroup\$