swup / preload-plugin

A swup plugin for preloading pages to speed up navigation 🚀
https://swup.js.org/plugins/preload-plugin
MIT License
14 stars 14 forks source link

Optimize `delegateListeners` #48

Closed hirasso closed 1 year ago

hirasso commented 1 year ago

Description

PR #47 introduced an issue where unmounting the plugin would throw an error, since based on context (desktop/touch) there ever was only one listener defined (either mouseover or touchstart).

At first, I solved it by making sure each listener was defined before destroying it. But then I realized that a better approach is to define both listeners initially, ignoring the context, and then return early in the handlers based on context:

onMouseOver = (event) => {
    // Return early on devices that don't support hover
    if (!window.matchMedia('(hover: hover)').matches) return;
    this.swup.triggerEvent('hoverLink', event);
    this.preloadLink(event.delegateTarget);
};

onTouchStart = (event) => {
    // Return early on devices that support hover
    if (window.matchMedia('(hover: hover)').matches) return;
    this.preloadLink(event.delegateTarget);
};

This makes it possible to change the behavior at runtime (think "mobile device mode" in the dev tools, for example). I realize that checking for hover: hover in touchstart is a bit weird, but you never know what crazy devices are out there :)

Additional information

No breaking changes.

daun commented 1 year ago

Nice! I'd just suggest extracting the check window.matchMedia('(hover: hover)').matches into a separate method with a descriptive name like isHoverSupported or isTouchDevice or similar.

hirasso commented 1 year ago

Of course :)

hirasso commented 1 year ago

Ready to go