Couple of fixes

CHANGES.md

@@ -34,6 +34,12 @@
 -   pat calendar: Store view, date and active categories per URL, allowing to individually customize the calendar per page.
 -   pat tooltip: Use tippy v6 based implementation.
 -   Allow overriding the public path from outside via the definition of a ``window.__patternslib_public_path__`` global variable.
+-   Introduce new ``core/dom`` module for DOM manipulation and traversing.
+    ``core/dom`` includes methods which help transition from jQuery to the JavaScript DOM API.
+-   core dom: Add ``jqToNode`` to return a DOM node if a jQuery node was passed.
+-   core dom: Add ``querySelectorAllAndMe`` to do a querySelectorAll including the starter element.
+-   core dom: Add ``wrap`` wrap an element with a wrapper element.
+-   core dom: Add ``hide`` and ``show`` for DOM elements which retain the original display value.
 
 ### Technical
 
@@ -61,6 +67,8 @@
 -   pat calendar: Fix language loading error "Error: Cannot find module './en.js'"
 -   pat depends, pat auto suggest: Fix a problem with initialization of ``pat-auto-suggest`` which occurred after the lazy loading changes.
 -   pat checklist: Also dispatch standard ``change`` event when de/selecting all items.
+-   pat select: Add missing ``<span>`` element around the select element itself. Fixes: https://github.com/quaive/ploneintranet.prototype/issues/1087
+-   pat depends/core utils: Do not set inline styles when showing elements in transition mode ``none``. Fixes #719.
 
 
 ## 3.0.0-dev - unreleased

src/core/dom.js

@@ -0,0 +1,66 @@
+/* Utilities for DOM traversal or navigation */
+
+const DATA_STYLE_DISPLAY = "__patternslib__style__display";
+
+const jqToNode = (el) => {
+    // Return a DOM node if a jQuery node was passed.
+    if (el.jquery) {
+        el = el[0];
+    }
+    return el;
+};
+
+const querySelectorAllAndMe = (el, selector) => {
+    // Like querySelectorAll but including the element where it starts from.
+    // Returns an Array, not a NodeList
+
+    el = jqToNode(el); // Ensure real DOM node.
+
+    const all = [...el.querySelectorAll(selector)];
+    if (el.matches(selector)) {
+        all.unshift(el); // start element should be first.
+    }
+    return all;
+};
+
+const wrap = (el, wrapper) => {
+    // Wrap a element with a wrapper element.
+    // See: https://stackoverflow.com/a/13169465/1337474
+    el = jqToNode(el); // Ensure real DOM node.
+    wrapper = jqToNode(wrapper); // Ensure real DOM node.
+
+    el.parentNode.insertBefore(wrapper, el);
+    wrapper.appendChild(el);
+};
+
+const hide = (el) => {
+    // Hides the element with ``display: none``
+    el = jqToNode(el); // Ensure real DOM node.
+    if (el.style.display === "none") {
+        // Nothing to do.
+        return;
+    }
+    if (el.style.display) {
+        el[DATA_STYLE_DISPLAY] = el.style.display;
+    }
+    el.style.display = "none";
+};
+
+const show = (el) => {
+    // Shows element by removing ``display: none`` and restoring the display
+    // value to whatever it was before.
+    el = jqToNode(el); // Ensure real DOM node.
+    const val = el[DATA_STYLE_DISPLAY] || null;
+    el.style.display = val;
+    delete el[DATA_STYLE_DISPLAY];
+};
+
+const dom = {
+    jqToNode: jqToNode,
+    querySelectorAllAndMe: querySelectorAllAndMe,
+    wrap: wrap,
+    hide: hide,
+    show: show,
+};
+
+export default dom;

src/core/dom.test.js

@@ -0,0 +1,108 @@
+import $ from "jquery";
+import dom from "./dom";
+
+describe("core.dom tests", () => {
+    // Tests from the core.dom module
+
+    describe("jqToNode tests", () => {
+        it("always returns a bare DOM node no matter if a jQuery or bare DOM node was passed.", (done) => {
+            const el = document.createElement("div");
+            const $el = $(el);
+
+            expect(dom.jqToNode($el)).toBe(el);
+            expect(dom.jqToNode(el)).toBe(el);
+
+            done();
+        });
+    });
+
+    describe("querySelectorAllAndMe tests", () => {
+        it("return also starting node if query matches.", (done) => {
+            const el = document.createElement("div");
+            el.setAttribute("class", "node1");
+            el.innerHTML = `<span class="node2">hello.</span>`;
+
+            const res1 = dom.querySelectorAllAndMe(el, ".node1");
+            expect(res1.length).toBe(1);
+            expect(res1[0].outerHTML).toBe(
+                `<div class="node1"><span class="node2">hello.</span></div>`
+            );
+
+            const res2 = dom.querySelectorAllAndMe(el, ".node2");
+            expect(res2.length).toBe(1);
+            expect(res2[0].outerHTML).toBe(`<span class="node2">hello.</span>`);
+
+            const res3 = dom.querySelectorAllAndMe(el, "div, span");
+            expect(res3.length).toBe(2);
+            expect(res3[0].outerHTML).toBe(
+                `<div class="node1"><span class="node2">hello.</span></div>`
+            );
+            expect(res3[1].outerHTML).toBe(`<span class="node2">hello.</span>`);
+
+            done();
+        });
+    });
+
+    describe("wrap tests", () => {
+        it("wraps an element within another element.", (done) => {
+            const parent = document.createElement("main");
+            const el = document.createElement("div");
+            const wrapper = document.createElement("section");
+            parent.appendChild(el);
+
+            dom.wrap(el, wrapper);
+            expect(parent.outerHTML).toBe(
+                `<main><section><div></div></section></main>`
+            );
+
+            done();
+        });
+    });
+
+    describe("show/hide tests", () => {
+        it("shows or hides and does keeps the CSS display rule value.", (done) => {
+            const el = document.createElement("div");
+            el.style.borderTop = "2em";
+            el.style.marginTop = "4em";
+
+            dom.hide(el);
+
+            expect(el.style.borderTop).toBe("2em");
+            expect(el.style.marginTop).toBe("4em");
+            expect(el.style.display).toBe("none");
+            expect(
+                el.getAttribute("style").indexOf("display") >= -1
+            ).toBeTruthy();
+
+            dom.show(el);
+
+            expect(el.style.borderTop).toBe("2em");
+            expect(el.style.marginTop).toBe("4em");
+            expect(el.style.display).toBeFalsy();
+            expect(
+                el.getAttribute("style").indexOf("display") === -1
+            ).toBeTruthy();
+
+            el.style.display = "inline";
+            dom.hide(el);
+
+            expect(el.style.borderTop).toBe("2em");
+            expect(el.style.marginTop).toBe("4em");
+            expect(el.style.display).toBe("none");
+            expect(
+                el.getAttribute("style").indexOf("display") >= -1
+            ).toBeTruthy();
+
+            dom.show(el);
+
+            expect(el.style.borderTop).toBe("2em");
+            expect(el.style.marginTop).toBe("4em");
+            expect(el.style.display).toBe("inline");
+            expect(
+                el.getAttribute("style").indexOf("display") >= -1
+            ).toBeTruthy();
+
+            done();
+        });
+    });
+});

src/core/utils.js

@@ -1,5 +1,6 @@
 import $ from "jquery";
 import _ from "underscore";
+import dom from "./dom";
 
 $.fn.safeClone = function () {
     var $clone = this.clone();
@@ -278,45 +279,53 @@ function hasValue(el) {
     return false;
 }
 
-const transitions = {
-    none: { hide: "hide", show: "show" },
-    fade: { hide: "fadeOut", show: "fadeIn" },
-    slide: { hide: "slideUp", show: "slideDown" },
-};
+const hideOrShow = (el, visible, options, pattern_name) => {
+    const $el = $(el);
+    el = dom.jqToNode(el); // ensure dom node
+
+    const transitions = {
+        none: { hide: "hide", show: "show" },
+        fade: { hide: "fadeOut", show: "fadeIn" },
+        slide: { hide: "slideUp", show: "slideDown" },
+    };
 
-function hideOrShow($el, visible, options, pattern_name) {
     const duration =
         options.transition === "css" || options.transition === "none"
             ? null
             : options.effect.duration;
 
-    $el.removeClass("visible hidden in-progress");
-    const onComplete = function () {
-        $el.removeClass("in-progress")
-            .addClass(visible ? "visible" : "hidden")
-            .trigger("pat-update", {
-                pattern: pattern_name,
-                transition: "complete",
-            });
+    const on_complete = () => {
+        el.classList.remove("in-progress");
+        el.classList.add(visible ? "visible" : "hidden");
+        $(el).trigger("pat-update", {
+            pattern: pattern_name,
+            transition: "complete",
+        });
     };
-    if (!duration) {
-        if (options.transition !== "css") {
-            $el[visible ? "show" : "hide"]();
-        }
-        onComplete();
-    } else {
+
+    el.classList.remove("visible");
+    el.classList.remove("hidden");
+    el.classList.remove("in-progress");
+
+    if (duration) {
         const t = transitions[options.transition];
-        $el.addClass("in-progress").trigger("pat-update", {
+        el.classList.add("in-progress");
+        $el.trigger("pat-update", {
             pattern: pattern_name,
             transition: "start",
         });
         $el[visible ? t.show : t.hide]({
             duration: duration,
             easing: options.effect.easing,
-            complete: onComplete,
+            complete: on_complete,
         });
+    } else {
+        if (options.transition !== "css") {
+            dom[visible ? "show" : "hide"](el);
+        }
+        on_complete();
     }
-}
+};
 
 function addURLQueryParameter(fullURL, param, value) {
     /* Using a positive lookahead (?=\=) to find the given parameter,

src/core/utils.test.js

@@ -427,6 +427,43 @@ describe("hideOrShow", function () {
             "hidden",
         ]);
     });
+
+    it("transition none does not mess up styles", (done) => {
+        const el = document.createElement("div");
+        el.style.borderTop = "2em";
+        el.style.marginTop = "4em";
+
+        utils.hideOrShow(el, false, { transition: "none" }, "noname");
+
+        expect(el.style.borderTop).toBe("2em");
+        expect(el.style.marginTop).toBe("4em");
+        expect(el.style.display).toBe("none");
+        expect(el.getAttribute("style").indexOf("display") >= -1).toBeTruthy();
+
+        utils.hideOrShow(el, true, { transition: "none" }, "noname");
+
+        expect(el.style.borderTop).toBe("2em");
+        expect(el.style.marginTop).toBe("4em");
+        expect(el.style.display).toBeFalsy();
+        expect(el.getAttribute("style").indexOf("display") === -1).toBeTruthy();
+
+        el.style.display = "inline";
+        utils.hideOrShow(el, false, { transition: "none" }, "noname");
+
+        expect(el.style.borderTop).toBe("2em");
+        expect(el.style.marginTop).toBe("4em");
+        expect(el.style.display).toBe("none");
+        expect(el.getAttribute("style").indexOf("display") >= -1).toBeTruthy();
+
+        utils.hideOrShow(el, true, { transition: "none" }, "noname");
+
+        expect(el.style.borderTop).toBe("2em");
+        expect(el.style.marginTop).toBe("4em");
+        expect(el.style.display).toBe("inline");
+        expect(el.getAttribute("style").indexOf("display") >= -1).toBeTruthy();
+
+        done();
+    });
 });
 
 describe("hasValue", function () {

src/pat/depends/index.html

@@ -21,19 +21,19 @@
                     <legend>Flavour</legend>
 
                     <label
-                        ><input type="radio" name="flavour" value="margherita" />
+                        ><input type="radio" name="flavour" value="margherita" autocomplete="off" />
                         Margherita</label
                     >
                     <label
-                        ><input type="radio" name="flavour" value="hawaii" />
+                        ><input type="radio" name="flavour" value="hawaii" autocomplete="off" />
                         Hawaii</label
                     >
                     <label
-                        ><input type="radio" name="flavour" value="bacon" />
+                        ><input type="radio" name="flavour" value="bacon" autocomplete="off" checked />
                         Bacon Supreme</label
                     >
                     <label
-                        ><input type="radio" name="flavour" value="sea" />
+                        ><input type="radio" name="flavour" value="sea" autocomplete="off" />
                         Fruits de mer</label
                     >
                 </fieldset>
@@ -55,7 +55,7 @@
                 <fieldset class="group pat-checklist">
                     <legend>Add extra toppings</legend>
                     <label>
-                        <input type="checkbox" name="custom" /> Add extra
+                        <input type="checkbox" name="custom" autocomplete="off" /> Add extra
                         toppings
                     </label>
                 </fieldset>
@@ -68,18 +68,18 @@
                     <label
                         class="pat-depends"
                         data-pat-depends="flavour!=hawaii"
-                        ><input type="checkbox" name="pineapple" />
+                        ><input type="checkbox" name="pineapple" autocomplete="off"/>
                         Pineapple</label
                     >
                     <label
-                        ><input type="checkbox" name="gorgonzola" />
+                        ><input type="checkbox" name="gorgonzola" autocomplete="off" />
                         Gorgonzola</label
                     >
                     <label
-                        ><input type="checkbox" name="peanuts" /> Peanuts</label
+                        ><input type="checkbox" name="peanuts" autocomplete="off" /> Peanuts</label
                     >
                     <label
-                        ><input type="checkbox" name="redonion" /> Red
+                        ><input type="checkbox" name="redonion" autocomplete="off" /> Red
                         onions</label
                     >
                 </fieldset>

src/pat/selectbox/documentation.md

@@ -0,0 +1,5 @@
+## Description
+
+the __selectbox__ pattern adds ``data-option`` and ``data-option-value`` attributes on the parent element.
+If the parent element is not a ``<label>`` it wrapts itself within a ``<span>`` element.
+