fix: [Performance] Improvements in Columnview (#185)

coral-base-button/src/scripts/BaseButton.js

@@ -215,10 +215,14 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
 
   set iconPosition(value) {
     value = transform.string(value).toLowerCase();
-    this._iconPosition = validate.enumeration(iconPosition)(value) && value || iconPosition.LEFT;
-    this._reflectAttribute('iconposition', this._iconPosition);
+    value = validate.enumeration(iconPosition)(value) && value || iconPosition.LEFT;
+
+    this._reflectAttribute('iconposition', value);
 
-    this._updateIcon(this.icon);
+    if(validate.valueMustChange(this._iconPosition, value)) {
+      this._iconPosition = value;
+      this._updateIcon(this.icon);
+    }
   }
 
   /**
@@ -237,9 +241,11 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
   }
 
   set icon(value) {
-    this._icon = transform.string(value);
-
-    this._updateIcon(value);
+    value = transform.string(value);
+    if(validate.valueMustChange(this._icon, value)) {
+      this._icon = value;
+      this._updateIcon(value);
+    }
   }
 
   /**
@@ -259,10 +265,11 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
 
   set iconSize(value) {
     value = transform.string(value).toUpperCase();
-    this._iconSize = validate.enumeration(Icon.size)(value) && value || Icon.size.SMALL;
+    value = validate.enumeration(Icon.size)(value) && value || Icon.size.SMALL;
 
-    if (this._updatedIcon) {
-      this._getIconElement().setAttribute('size', value);
+    if(validate.valueMustChange(this._iconSize, value)) {
+      this._iconSize = value;
+      this._updatedIcon && this._getIconElement().setAttribute('size', value);
     }
   }
 
@@ -283,10 +290,11 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
 
   set iconAutoAriaLabel(value) {
     value = transform.string(value).toLowerCase();
-    this._iconAutoAriaLabel = validate.enumeration(Icon.autoAriaLabel)(value) && value || Icon.autoAriaLabel.OFF;
+    value = validate.enumeration(Icon.autoAriaLabel)(value) && value || Icon.autoAriaLabel.OFF;
 
-    if (this._updatedIcon) {
-      this._getIconElement().setAttribute('autoarialabel', value);
+    if(validate.valueMustChange(this._iconAutoAriaLabel, value)) {
+      this._iconAutoAriaLabel = value;
+      this._updatedIcon && this._getIconElement().setAttribute('autoarialabel', value);
     }
   }
 
@@ -306,6 +314,7 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
   set size(value) {
     value = transform.string(value).toUpperCase();
     this._size = validate.enumeration(size)(value) && value || size.MEDIUM;
+
     this._reflectAttribute('size', this._size);
   }
 
@@ -322,12 +331,17 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
   }
 
   set selected(value) {
-    this._selected = transform.booleanAttr(value);
-    this._reflectAttribute('selected', this._selected);
+    value = transform.booleanAttr(value);
+
+    this._reflectAttribute('selected', value);
 
-    this.classList.toggle('is-selected', this._selected);
+    if(validate.valueMustChange(this._selected, value)) {
+      this._selected = value;
 
-    this.trigger('coral-button:_selectedchanged');
+      this.classList.toggle('is-selected', value);
+
+      this.trigger('coral-button:_selectedchanged');
+    }
   }
 
   // We just reflect it but we also trigger an event to be used by button group
@@ -355,10 +369,15 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
   }
 
   set block(value) {
-    this._block = transform.booleanAttr(value);
-    this._reflectAttribute('block', this._block);
+    value = transform.booleanAttr(value);
+
+    this._reflectAttribute('block', value);
+
+    if(validate.valueMustChange(this._block, value)) {
+      this._block = value;
 
-    this.classList.toggle(`${CLASSNAME}--block`, this._block);
+      this.classList.toggle(`${CLASSNAME}--block`, value);
+    }
   }
 
   /**
@@ -375,25 +394,30 @@ const BaseButton = (superClass) => class extends BaseLabellable(superClass) {
 
   set variant(value) {
     value = transform.string(value).toLowerCase();
-    this._variant = validate.enumeration(variant)(value) && value || variant.DEFAULT;
-    this._reflectAttribute('variant', this._variant);
+    value = validate.enumeration(variant)(value) && value || variant.DEFAULT;
 
-    // removes every existing variant
-    this.classList.remove(CLASSNAME, ACTION_CLASSNAME);
-    this.classList.remove(...ALL_VARIANT_CLASSES);
+    this._reflectAttribute('variant', value);
 
-    if (this._variant === variant._CUSTOM) {
-      this.classList.remove(CLASSNAME);
-    } else {
-      this.classList.add(...VARIANT_MAP[this._variant]);
+    if(validate.valueMustChange(this._variant , value)) {
+      this._variant = value;
 
-      if (this._variant === variant.ACTION || this._variant === variant.QUIET_ACTION) {
+      // removes every existing variant
+      this.classList.remove(CLASSNAME, ACTION_CLASSNAME);
+      this.classList.remove(...ALL_VARIANT_CLASSES);
+
+      if (value === variant._CUSTOM) {
         this.classList.remove(CLASSNAME);
+      } else {
+        this.classList.add(...VARIANT_MAP[value]);
+
+        if (value === variant.ACTION || value === variant.QUIET_ACTION) {
+          this.classList.remove(CLASSNAME);
+        }
       }
+
+      // Update label styles
+      this._updateLabel();
     }
-
-    // Update label styles
-    this._updateLabel();
   }
 
   /**

coral-base-component/src/scripts/BaseComponent.js

@@ -425,7 +425,6 @@ const BaseComponent = (superClass) => class extends superClass {
   _delegateEvents(eventMap) {
     this._events = commons.extend(this._events, eventMap);
     delegateEvents.call(this);
-    delegateGlobalEvents.call(this);
 
     // Once events are attached, we dispose them
     this._events = {};
@@ -844,9 +843,10 @@ const BaseComponent = (superClass) => class extends superClass {
   /** @ignore */
   connectedCallback() {
     // A component that is reattached should respond to global events again
-    if (this._disconnected) {
-      delegateGlobalEvents.call(this);
-    }
+    // Attach global listener when component is connected to DOM
+    // this would avoid memory leak when element is created but never connected.
+    delegateGlobalEvents.call(this);
+
     this._disconnected = false;
 
     if (!this._rendered) {
@@ -863,6 +863,7 @@ const BaseComponent = (superClass) => class extends superClass {
   disconnectedCallback() {
     // A component that isn't in the DOM should not be responding to global events
     this._disconnected = true;
+
     undelegateGlobalEvents.call(this);
   }
 };

coral-base-fieldgroup/src/scripts/BaseFieldGroup.js

@@ -68,10 +68,15 @@ const BaseFieldGroup = (superClass) => class extends superClass {
 
   set orientation(value) {
     value = transform.string(value).toLowerCase();
-    this._orientation = validate.enumeration(this.constructor.orientation)(value) && value || orientation.HORIZONTAL;
-    this._reflectAttribute('orientation', this._orientation);
-
-    this.classList.toggle(`${CLASSNAME}--vertical`, this._orientation === orientation.VERTICAL);
+    value = validate.enumeration(this.constructor.orientation)(value) && value || orientation.HORIZONTAL;
+
+    this._reflectAttribute('orientation', value);
+
+    if(validate.valueMustChange(this._orientation, value)) {
+      this._orientation = value;
+
+      this.classList.toggle(`${CLASSNAME}--vertical`, value === orientation.VERTICAL);
+    }
   }
 
   /**

coral-base-formfield/src/scripts/BaseFormField.js

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {transform, commons} from '../../../coral-utils';
+import {transform, commons, validate} from '../../../coral-utils';
 
 // https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories
 let LABELLABLE_ELEMENTS_SELECTOR = 'button,input:not([type=hidden]),keygen,meter,output,progress,select,textarea';
@@ -145,11 +145,15 @@ const BaseFormField = (superClass) => class extends superClass {
   }
 
   set invalid(value) {
-    this._invalid = transform.booleanAttr(value);
-    this._reflectAttribute('invalid', this._invalid);
-
-    this.setAttribute('aria-invalid', this._invalid);
-    this.classList.toggle('is-invalid', this._invalid);
+    value = transform.booleanAttr(value);
+
+    this._reflectAttribute('invalid', value);
+
+    if(validate.valueMustChange(this._invalid, value)) {
+      this._invalid = value;
+      this.setAttribute('aria-invalid', value);
+      this.classList.toggle('is-invalid', value);
+    }
   }
 
   /**

coral-base-list/src/scripts/BaseList.js

@@ -94,6 +94,7 @@ const BaseList = (superClass) => class extends superClass {
   set interaction(value) {
     value = transform.string(value).toLowerCase();
     this._interaction = validate.enumeration(interaction)(value) && value || interaction.ON;
+
     this._reflectAttribute('interaction', this._interaction);
   }
 

coral-base-list/src/scripts/BaseListItem.js

@@ -12,7 +12,7 @@
 
 import {BaseLabellable} from '../../../coral-base-labellable';
 import {Icon} from '../../../coral-component-icon';
-import {transform} from '../../../coral-utils';
+import {transform, validate} from '../../../coral-utils';
 
 const CLASSNAME = '_coral-Menu-item';
 
@@ -69,11 +69,14 @@ const BaseListItem = (superClass) => class extends BaseLabellable(superClass) {
   }
 
   set disabled(value) {
-    this._disabled = transform.booleanAttr(value);
-    this._reflectAttribute('disabled', this._disabled);
-
-    this.classList.toggle('is-disabled', this._disabled);
-    this[this._disabled ? 'setAttribute' : 'removeAttribute']('aria-disabled', this._disabled);
+    value = transform.booleanAttr(value);
+    this._reflectAttribute('disabled', value);
+
+    if(validate.valueMustChange(this._disabled, value)) {
+      this._disabled = value;
+      this.classList.toggle('is-disabled', value);
+      this[value ? 'setAttribute' : 'removeAttribute']('aria-disabled', value);
+    }
   }
 
   /**

coral-base-overlay/src/scripts/BaseOverlay.js

@@ -431,6 +431,7 @@ const BaseOverlay = (superClass) => class extends superClass {
       this._handleRootKeypress = this._handleRootKeypress.bind(this);
       this._vent.on('keydown', this._handleRootKeypress);
       this._vent.on('focus', '[coral-tabcapture]', this._handleTabCaptureFocus);
+
     } else if (this._trapFocus === trapFocus.OFF) {
       // Remove elements
       this._elements.topTabCapture && this._elements.topTabCapture.remove();

coral-component-actionbar/src/scripts/ActionBar.js

@@ -558,10 +558,10 @@ const ActionBar = Decorator(class extends BaseComponent(HTMLElement) {
     const primary = this._elements.primary;
     const secondary = this._elements.secondary;
 
-    if (!primary.getAttribute('role')) {
+    if (!primary.hasAttribute('role')) {
       primary.setAttribute('role', 'toolbar');
     }
-    if (!secondary.getAttribute('role')) {
+    if (!secondary.hasAttribute('role')) {
       secondary.setAttribute('role', 'toolbar');
     }
 

coral-component-columnview/src/scripts/ColumnView.js

@@ -169,22 +169,28 @@ const ColumnView = Decorator(class extends BaseComponent(HTMLElement) {
 
   set selectionMode(value) {
     value = transform.string(value).toLowerCase();
-    this._selectionMode = validate.enumeration(selectionMode)(value) && value || selectionMode.NONE;
-    this._reflectAttribute('selectionmode', this._selectionMode);
+    value = validate.enumeration(selectionMode)(value) && value || selectionMode.NONE;
 
-    // propagates the selection mode to the columns
-    this.columns.getAll().forEach((item) => {
-      item.setAttribute('_selectionmode', this._selectionMode);
-    });
-
-    this.classList.remove(`${CLASSNAME}--selection`);
+    this._reflectAttribute('selectionmode', value);
+
+    if(validate.valueMustChange(this._selectionMode, value)) {
+      this._selectionMode = value;
 
-    if (this._selectionMode !== selectionMode.NONE) {
-      this.classList.add(`${CLASSNAME}--selection`);
+      // propagates the selection mode to the columns
+      let columns = this.columns.getAll();
+      columns.forEach((item) => {
+        item.setAttribute('_selectionmode', value);
+      });
+
+      this.classList.remove(`${CLASSNAME}--selection`);
+
+      if (value !== selectionMode.NONE) {
+        this.classList.add(`${CLASSNAME}--selection`);
+      }
+
+      // @a11y
+      this.setAttribute('aria-multiselectable', value === selectionMode.MULTIPLE);
     }
-
-    // @a11y
-    this.setAttribute('aria-multiselectable', this._selectionMode === selectionMode.MULTIPLE);
   }
 
   /**
@@ -819,9 +825,10 @@ const ColumnView = Decorator(class extends BaseComponent(HTMLElement) {
     const colIndex = this.columns.getAll().indexOf(column);
     const level = colIndex + 1;
     if (column.items) {
-      column.items.getAll().filter((item, index) => {
+      let items = column.items.getAll();
+      items.filter((item, index) => {
         item.setAttribute('aria-posinset', index + 1);
-        item.setAttribute('aria-setsize', column.items.length);
+        item.setAttribute('aria-setsize', items.length);
         return !item.hasAttribute('aria-level');
       }).forEach((item) => {
         item.setAttribute('aria-level', level);
@@ -1127,7 +1134,7 @@ const ColumnView = Decorator(class extends BaseComponent(HTMLElement) {
 
     // @a11y append live region content element
     if (!this.contains(accessibilityState)) {
-      this.insertBefore(accessibilityState, this.firstChild);
+      this.appendChild(accessibilityState);
     }
 
     // utility method to clean up accessibility state
@@ -1174,7 +1181,9 @@ const ColumnView = Decorator(class extends BaseComponent(HTMLElement) {
       // give screen reader 2 secs before clearing the live region, to provide enough time for announcement
       this._removeTimeout = window.setTimeout(() => {
         resetAccessibilityState();
-        this._elements.accessibilityState = accessibilityState.parentNode.removeChild(accessibilityState);
+        if(accessibilityState.parentNode) {
+          this._elements.accessibilityState = accessibilityState.parentNode.removeChild(accessibilityState);
+        }
       }, 2000);
     }, 20);
   }

coral-component-columnview/src/scripts/ColumnViewColumn.js

@@ -173,12 +173,16 @@ const ColumnViewColumn = Decorator(class extends BaseComponent(HTMLElement) {
 
   set _selectionMode(value) {
     value = transform.string(value).toLowerCase();
-    this.__selectionMode = validate.enumeration(selectionMode)(value) && value || null;
-    this._reflectAttribute('_selectionmode', this.__selectionMode);
-
-    this.items.getAll().forEach(item => this._toggleItemSelection(item));
-
-    this._setStateFromDOM();
+    value = validate.enumeration(selectionMode)(value) && value || null;
+
+    this._reflectAttribute('_selectionmode', value);
+
+    if(validate.valueMustChange(this.__selectionMode, value)) {
+      this.__selectionMode = value;
+      let items = this.items.getAll();
+      items.forEach(item => this._toggleItemSelection(item));
+      this._setStateFromDOM();
+    }
   }
 
   /**