changed: code improvements for getSiblingElements calls to improve pe…

…rformance,

OpenClinica/enketo-express-oc#301

src/js/dom-utils.js

@@ -7,7 +7,7 @@
  *
  * @static
  * @param {Node} element - Target element.
- * @param {string} [selector] - A CSS selector.
+ * @param {string} [selector] - A CSS selector for siblings (not for self).
  * @return {Array<Node>} Array of sibling nodes plus target element.
  */
 function getSiblingElementsAndSelf( element, selector ) {
@@ -26,6 +26,27 @@ function getSiblingElements( element, selector ) {
     return _getSiblingElements( element, selector );
 }
 
+/**
+ * Returns first sibling element (in DOM order) that optionally matches the provided selector.
+ *
+ * @param {Node} element - Target element.
+ * @param {string} [selector] - A CSS selector.
+ * @return {Node} First sibling element in DOM order
+ */
+function getSiblingElement( element, selector = '*' ){
+    let found;
+    let current = element.parentElement.firstElementChild;
+
+    while ( current && !found ) {
+        if ( current !== element && current.matches( selector ) ) {
+            found = current;
+        }
+        current = current.nextElementSibling;
+    }
+
+    return found;
+}
+
 /**
  * Gets siblings that match selector _in DOM order_.
  *
@@ -38,7 +59,7 @@ function _getSiblingElements( element, selector = '*', startArray = [] ) {
     const siblings = startArray;
     let prev = element.previousElementSibling;
     let next = element.nextElementSibling;
-
+    // TODO: check if iteration approach used by getSiblingElement is faster. It would be more elegant.
     while ( prev ) {
         if ( prev.matches( selector ) ) {
             siblings.unshift( prev );
@@ -103,11 +124,25 @@ function closestAncestorUntil( element, filterSelector = '*', endSelector ) {
     return found;
 }
 
+/**
+ * Gets child elements, that (optionally) match a selector.
+ *
+ * @param {Node} element - Target element.
+ * @param {string} selector - A CSS selector.
+ * @return {Array<Node>} Array of child elements.
+ */
 function getChildren( element, selector = '*' ) {
     return [ ...element.children ]
         .filter( el => el.matches( selector ) );
 }
 
+/**
+ * Gets first child element, that (optionally) matches a selector.
+ *
+ * @param {Node} element - Target element.
+ * @param {string} selector - A CSS selector.
+ * @return {Node} - First child element.
+ */
 function getChild( element, selector = '*' ) {
     return [ ...element.children ]
         .find( el => el.matches( selector ) );
@@ -382,6 +417,7 @@ export {
     elementDataStore,
     getSiblingElementsAndSelf,
     getSiblingElements,
+    getSiblingElement,
     getAncestors,
     getChildren,
     getChild,

src/js/form.js

@@ -1,7 +1,7 @@
 import { FormModel } from './form-model';
 import $ from 'jquery';
 import { parseFunctionFromExpression, stripQuotes, getFilename, joinPath } from './utils';
-import { getXPath, getChild, closestAncestorUntil, getSiblingElements } from './dom-utils';
+import { getXPath, getChild, closestAncestorUntil, getSiblingElement } from './dom-utils';
 import { t } from 'enketo/translator';
 import config from 'enketo/config';
 import inputHelper from './input';
@@ -425,19 +425,19 @@ Form.prototype.replaceChoiceNameFn = function( expr, resTypeStr, context, index,
             if ( !value || !inputs.length ) {
                 label = '';
             } else if (  nodeName === 'select' ) {
-                const found = inputs.filter( input => input.querySelector( `[value="${value}"]` ) );
-                label =  found.length ? found[0].querySelector( `[value="${value}"]` ).textContent : '';
+                const found = inputs.find( input => input.querySelector( `[value="${value}"]` ) );
+                label =  found ? found.querySelector( `[value="${value}"]` ).textContent : '';
             } else if (  nodeName === 'input' ) {
                 const list = inputs[0].getAttribute( 'list' );
 
                 if ( !list ){
-                    const found = inputs.filter( input => input.getAttribute( 'value' ) === value );
-                    const siblingLabelEls = found.length ? getSiblingElements( found[0], '.option-label.active' ) : [];
-                    label = siblingLabelEls.length ? siblingLabelEls[0].textContent : '';
+                    const found = inputs.find( input => input.getAttribute( 'value' ) === value );
+                    const firstSiblingLabelEl = found ? getSiblingElement( found, '.option-label.active' ) : [];
+                    label = firstSiblingLabelEl ? firstSiblingLabelEl.textContent : '';
                 } else {
-                    const siblingListEls = getSiblingElements( inputs[0], `datalist#${CSS.escape( list )}` );
-                    if ( siblingListEls.length ){
-                        const optionEl = siblingListEls[0].querySelector( `[data-value="${value}"]` );
+                    const firstSiblingListEl = getSiblingElement( inputs[0], `datalist#${CSS.escape( list )}` );
+                    if ( firstSiblingListEl ){
+                        const optionEl = firstSiblingListEl.querySelector( `[data-value="${value}"]` );
                         label = optionEl ? optionEl.getAttribute( 'value' ) : '';
                     }
                 }

src/js/itemset.js

@@ -6,7 +6,7 @@
 
 import { parseFunctionFromExpression } from './utils';
 import dialog from 'enketo/dialog';
-import { closestAncestorUntil, getChild, getSiblingElements, elementDataStore as data } from './dom-utils';
+import { closestAncestorUntil, getChild, getSiblingElement, elementDataStore as data } from './dom-utils';
 import events from './event';
 import { t } from 'enketo/translator';
 
@@ -94,19 +94,19 @@ export default {
                     inputAttributes[ attr.name ] = attr.value;
                 } );
                 // If this is a ranking widget:
-                input = optionInput.classList.contains( 'ignore' ) ? getSiblingElements( optionInput.closest( '.option-wrapper' ), 'input.rank' )[ 0 ] : optionInput;
+                input = optionInput.classList.contains( 'ignore' ) ? getSiblingElement( optionInput.closest( '.option-wrapper' ), 'input.rank' ) : optionInput;
             } else if ( list && list.nodeName.toLowerCase() === 'select' ) {
                 input = list;
             } else if ( list && list.nodeName.toLowerCase() === 'datalist' ) {
                 if ( shared ) {
                     // only the first input, is that okay?
                     input = that.form.view.html.querySelector( `input[name="${list.dataset.name}"]` );
                 } else {
-                    input = getSiblingElements( list, 'input:not(.widget)' )[ 0 ];
+                    input = getSiblingElement( list, 'input:not(.widget)' );
                 }
             }
 
-            const labelsContainer = getSiblingElements( template.closest( 'label, select, datalist' ), '.itemset-labels' )[ 0 ];
+            const labelsContainer = getSiblingElement( template.closest( 'label, select, datalist' ), '.itemset-labels' );
             const itemsXpath = template.dataset.itemsPath;
             let labelType = labelsContainer.dataset.labelType;
             let labelRef = labelsContainer.dataset.labelRef;

src/js/language.js

@@ -4,7 +4,7 @@
  * @module language
  */
 
-import { getSiblingElements } from './dom-utils';
+import { getSiblingElement } from './dom-utils';
 import events from './event';
 
 export default {
@@ -91,7 +91,7 @@ export default {
         translations.forEach( el => el.classList.remove( 'active' ) );
         translations
             .filter( el => el.matches( `[lang="${lang}"], [lang=""]` ) &&
-                ( !el.classList.contains( 'or-form-short' ) || ( el.classList.contains( 'or-form-short' ) && getSiblingElements( el, '.or-form-long' ).length === 0 ) ) )
+                ( !el.classList.contains( 'or-form-short' ) || ( el.classList.contains( 'or-form-short' ) && !getSiblingElement( el, '.or-form-long' ) ) ) )
             .forEach( el => el.classList.add(
                 'active'
             ) );

src/js/page.js

@@ -7,7 +7,7 @@
 import $ from 'jquery';
 import events from './event';
 import config from 'enketo/config';
-import { getSiblingElements, getAncestors } from './dom-utils';
+import { getSiblingElement, getAncestors } from './dom-utils';
 import 'jquery-touchswipe';
 
 export default {
@@ -264,7 +264,7 @@ export default {
                     // or-repeat-info is only considered a page by itself if it has no sibling repeats
                     // When there are siblings repeats, we use CSS trickery to show the + button underneath the last
                     // repeat.
-                    ( el.matches( '.or-repeat-info' ) && getSiblingElements( el, '.or-repeat' ).length === 0 ) );
+                    ( el.matches( '.or-repeat-info' ) && !getSiblingElement( el, '.or-repeat' ) ) );
         } );
         this._updateToc();
     },

src/js/repeat.js

@@ -15,7 +15,7 @@ import $ from 'jquery';
 import events from './event';
 import { t } from 'enketo/translator';
 import dialog from 'enketo/dialog';
-import { getSiblingElements, getChildren, getSiblingElementsAndSelf } from './dom-utils';
+import { getSiblingElements, getSiblingElement, getChildren, getSiblingElementsAndSelf } from './dom-utils';
 import { isStaticItemsetFromSecondaryInstance } from './itemset';
 import config from 'enketo/config';
 const disableFirstRepeatRemoval = config.repeatOrdinals === true;
@@ -390,8 +390,7 @@ export default {
                 this.fixDatalistId( datalist );
             } else {
                 const id = datalist.id;
-                const inputs = getSiblingElements( datalist, 'input[list]' );
-                const input = inputs.length ? inputs[ 0 ] : null;
+                const input = getSiblingElement( datalist, 'input[list]' );
 
                 if ( input ) {
                     // For very long static datalists, a huge performance improvement can be achieved, by using the

src/js/toc.js

@@ -4,7 +4,7 @@
  * @module toc
  */
 
-import { getAncestors, getSiblingElements } from './dom-utils';
+import { getAncestors, getSiblingElement } from './dom-utils';
 
 export default {
     /**
@@ -29,7 +29,7 @@ export default {
                         // or-repeat-info is only considered a page by itself if it has no sibling repeats
                         // When there are siblings repeats, we use CSS trickery to show the + button underneath the last
                         // repeat.
-                        ( tocEl.matches( '.or-repeat-info' ) && getSiblingElements( tocEl, '.or-repeat' ).length === 0 ) );
+                        ( tocEl.matches( '.or-repeat-info' ) && !getSiblingElement( tocEl, '.or-repeat' ) ) );
             } )
             .filter( tocEl => !tocEl.classList.contains( 'or-repeat-info' ) );
         tocElements.forEach( ( element, index ) => {

src/widget/image-map/image-map.js

@@ -1,7 +1,7 @@
 import Widget from '../../js/widget';
 import { t } from 'enketo/translator';
 import events from '../../js/event';
-import { getSiblingElements } from '../../js/dom-utils';
+import { getSiblingElement } from '../../js/dom-utils';
 const SELECTORS = 'path[id], g[id], circle[id]';
 
 /**
@@ -203,8 +203,8 @@ class ImageMap extends Widget {
         this.svg.querySelectorAll( SELECTORS ).forEach( el => {
             el.addEventListener( 'mouseenter', ev => {
                 const id = ev.target.id || ev.target.closest( 'g[id]' ).id;
-                const labels = getSiblingElements( this._getInput( id ), '.option-label.active' );
-                const optionLabel = labels && labels.length ? labels[0].textContent : '';
+                const label = getSiblingElement( this._getInput( id ), '.option-label.active' );
+                const optionLabel = label ? label.textContent : '';
                 this.tooltip.textContent = optionLabel;
             } );
             el.addEventListener( 'mouseleave', ev => {

src/widget/select-autocomplete/autocomplete.js

@@ -1,7 +1,7 @@
 import $ from 'jquery';
 import Widget from '../../js/widget';
 import events from '../../js/event';
-import { getSiblingElements } from '../../js/dom-utils';
+import { getSiblingElement } from '../../js/dom-utils';
 
 const sadExcuseForABrowser = !( 'list' in document.createElement( 'input' ) &&
     'options' in document.createElement( 'datalist' ) &&
@@ -32,9 +32,9 @@ class AutocompleteSelectpicker extends Widget {
     _init() {
         const listId = this.element.getAttribute( 'list' );
 
-        if ( getSiblingElements( this.element, 'datalist' ).length === 0 ) {
-            const infos = getSiblingElements( this.element.closest( '.or-repeat' ), '.or-repeat-info' );
-            this.options = infos.length ? [ ...infos[ 0 ].querySelectorAll( `datalist#${CSS.escape( listId )} > option` ) ] : [];
+        if ( !getSiblingElement( this.element, 'datalist' ) ) {
+            const info = getSiblingElement( this.element.closest( '.or-repeat' ), '.or-repeat-info' );
+            this.options = info ? [ ...info.querySelectorAll( `datalist#${CSS.escape( listId )} > option` ) ] : [];
         } else {
             this.options = [ ...this.question.querySelectorAll( `datalist#${CSS.escape( listId )} > option` ) ];
         }

src/widget/select-media/select-media.js

@@ -1,5 +1,5 @@
 import Widget from '../../js/widget';
-import { getSiblingElements } from '../../js/dom-utils';
+import { getSiblingElement } from '../../js/dom-utils';
 
 /**
  * Media Picker. Hides text labels if a media label is present.
@@ -16,7 +16,7 @@ class MediaPicker extends Widget {
 
     _init() {
         this.element.querySelectorAll( '.option-label' ).forEach( function( optionLabel ) {
-            if ( getSiblingElements( optionLabel, 'img, video, audio' ).length > 0 ) {
+            if ( getSiblingElement( optionLabel, 'img, video, audio' ) ) {
                 optionLabel.style.display = 'none';
             }
         } );

test/spec/dom-utils.spec.js

@@ -1,4 +1,4 @@
-import { getSiblingElements, getSiblingElementsAndSelf, getAncestors, closestAncestorUntil, getChildren, getChild, getXPath } from '../../src/js/dom-utils';
+import { getSiblingElements, getSiblingElementsAndSelf, getSiblingElement, getAncestors, closestAncestorUntil, getChildren, getChild, getXPath } from '../../src/js/dom-utils';
 
 function getFragment( htmlStr ) {
     return document.createRange().createContextualFragment( htmlStr );
@@ -68,6 +68,40 @@ describe( 'DOM utils', () => {
         } );
     } );
 
+    describe( 'hasSiblingElement', () => {
+        const fragment = getFragment( `
+            <root>
+                <div id="e">
+                    <div id="d" class="or b"></div>
+                    <div id="c" class="a something disabled"></div>
+                    <div id="b" class="a"></div>
+                    <div id="a" class="b"></div>
+                </div>
+            </root>
+        ` );
+
+        const e = fragment.querySelector( '#e' );
+        const d = fragment.querySelector( '#d' );
+        const c = fragment.querySelector( '#c' );
+        const b = fragment.querySelector( '#b' );
+        const a = fragment.querySelector( '#a' );
+
+        [
+            [ getSiblingElement( a ), d ],
+            [ getSiblingElement( a, '.a' ), c ],
+            [ getSiblingElement( a, '#a' ), undefined ],
+            [ getSiblingElement( a, '.b' ), d ],
+            [ getSiblingElement( d ), c ],
+            [ getSiblingElement( d, '#b' ), b ],
+            [ getSiblingElement( e ),  undefined ],
+            [ getSiblingElement( e, '.b' ), undefined ],
+        ].forEach( t => {
+            it( 'works', () => {
+                expect( t[ 0 ] ).toEqual( t[ 1 ] );
+            } );
+        } );
+    } );
+
     describe( 'getChildren', () => {
         const fragment = getFragment( `
             <root>