Skip to content

Commit

Permalink
fix: Performance Improvement - Masonry takes time to load with huge q…
Browse files Browse the repository at this point in the history
…uickactions items. (#163)

Co-authored-by: Pareesh Gupta <paregupt@adobe.com>
  • Loading branch information
Pareesh and Pareesh Gupta committed Jun 7, 2021
1 parent d9edfc3 commit 40aa149
Show file tree
Hide file tree
Showing 39 changed files with 1,206 additions and 261 deletions.
65 changes: 59 additions & 6 deletions coral-base-component/src/scripts/BaseComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,25 @@ const getConstructorName = function (constructor) {
return originalConstructor._componentName;
};

/**
This will recursively change the ignoreConnectedCallback
value for coral-components, since a parent has ignored
the callback all its items should ignore the callback hooks
@ignore
*/
const _recursiveIgnoreConnectedCallback = function(el, value) {
let children = el.children;
for (let i = 0; i < children.length; i++) {
let child = children[i];
// todo better check for coral-component
if(typeof child._ignoreConnectedCallback === 'boolean') {
child._ignoreConnectedCallback = value;
} else {
_recursiveIgnoreConnectedCallback(child, value);
}
}
};

/**
@base BaseComponent
@classdesc The base element for all Coral components
Expand Down Expand Up @@ -735,13 +754,37 @@ const BaseComponent = (superClass) => class extends superClass {
}

/**
* checks whether connectedCallback needs to be executed or not ,skip if component is not in connected state
* or connectedCallback already executed for the component or we are ignore the connectedCallback for some reason
*
* @returns {Boolean} return true for skipped cases
This should be executed when messenger is connect event is connected.
It will add the parent as a listener in child messenger.
@ignore
*/
_onMessengerConnected(event) {
event.stopImmediatePropagation();

let handler = event.detail.handler;
if(typeof handler === 'function') {
handler(this);
} else {
throw new Error("Messenger handler should be a function");
}
}

/**
specify whether the connected and disconnected hooks are ignore for component
@returns true when ignored
@private
*/
_skipConnectedCallback() {
return !this.isConnected || this._disconnected === false || this._ignoreConnectedCallback === true;
get _ignoreConnectedCallback() {
return this.__ignoreConnectedCallback || false;
}

set _ignoreConnectedCallback(value) {
value = transform.booleanAttr(value);

if(value !== this.__ignoreConnectedCallback) {
this.__ignoreConnectedCallback = value;
_recursiveIgnoreConnectedCallback(this, value);
}
}

/**
Expand Down Expand Up @@ -780,6 +823,16 @@ const BaseComponent = (superClass) => class extends superClass {
}
}

/**
called when we need to re-initialise things, when
connected/disconnected callback are skipped.
@param connected, true when element connectedcallback is getting skipped, else false
@private
*/
_updateCallback(connected) {
// do nothing
}

/** @ignore */
connectedCallback() {
// A component that is reattached should respond to global events again
Expand Down
2 changes: 0 additions & 2 deletions coral-component-buttongroup/src/scripts/ButtonGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -854,8 +854,6 @@ class ButtonGroup extends BaseFormField(BaseComponent(HTMLElement)) {
render() {
super.render();

super.connectedCallback();

this.classList.add(CLASSNAME);

// Default reflected attributes
Expand Down
4 changes: 3 additions & 1 deletion coral-component-card/src/scripts/Card.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,9 @@ class Card extends BaseComponent(HTMLElement) {

// In case a lot of alerts are added, they will not overflow the card
// Also check whether any alerts are available
this.classList.toggle(`${CLASSNAME}--overflow`, this.info.childNodes.length && this.info.scrollHeight > this.clientHeight);
requestAnimationFrame(()=> {
this.classList.toggle(`${CLASSNAME}--overflow`, this.info.childNodes.length && this.info.scrollHeight > this.clientHeight);
});
}
}

Expand Down
4 changes: 2 additions & 2 deletions coral-component-colorinput/src/scripts/ColorInputSlider.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* governing permissions and limitations under the License.
*/

import {Slider} from '../../../coral-component-slider';
import {ExtensibleSlider, Slider} from '../../../coral-component-slider';
import '../../../coral-component-tooltip';
import sliderBase from '../templates/sliderBase';
import {transform} from '../../../coral-utils';
Expand All @@ -23,7 +23,7 @@ const CLASSNAMES = ['_coral-ColorInput-slider', '_coral-Slider--color'];
@htmltag coral-colorinput-slider
@extends {Slider}
*/
class ColorInputSlider extends Slider {
class ColorInputSlider extends ExtensibleSlider {
/**
The gradient shown as slider background as space separated values (at least 2 values needed).
e.g: #ff0000 #ffff00 #00ff00 #00ffff #0000ff #ff00ff #ff0000
Expand Down
18 changes: 0 additions & 18 deletions coral-component-dialog/src/scripts/Dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -686,15 +686,6 @@ class Dialog extends BaseOverlay(BaseComponent(HTMLElement)) {
]);
}

/** @ignore */
connectedCallback() {
if (this._ignoreConnectedCallback) {
return;
}

super.connectedCallback();
}

/** @ignore */
render() {
super.render();
Expand Down Expand Up @@ -805,15 +796,6 @@ class Dialog extends BaseOverlay(BaseComponent(HTMLElement)) {
this.footer = footer;
this.content = content;
}

/** @ignore */
disconnectedCallback() {
if (this._ignoreConnectedCallback) {
return;
}

super.disconnectedCallback();
}
}

export default Dialog;
34 changes: 24 additions & 10 deletions coral-component-masonry/src/scripts/Masonry.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {BaseComponent} from '../../../coral-base-component';
import MasonryItem from './MasonryItem';
import {SelectableCollection} from '../../../coral-collection';
import {validate, transform, commons} from '../../../coral-utils';
import {Decorator} from '../../../coral-decorator';

const CLASSNAME = '_coral-Masonry';

Expand Down Expand Up @@ -113,7 +114,7 @@ const getPreviousItem = (item) => {
@extends {HTMLElement}
@extends {BaseComponent}
*/
class Masonry extends BaseComponent(HTMLElement) {
const Masonry = Decorator(class extends BaseComponent(HTMLElement) {
/** @ignore */
constructor() {
super();
Expand Down Expand Up @@ -155,9 +156,8 @@ class Masonry extends BaseComponent(HTMLElement) {
'click coral-masonry-item': '_onItemClick',
'key:space coral-masonry-item': '_onItemClick',

// private
'coral-masonry-item:_connected': '_onItemConnected',
'coral-masonry-item:_selectedchanged': '_onItemSelectedChanged'
// Messenger
'coral-masonry-item:_messengerconnected': '_onMessengerConnected'
});

// Relayout when child elements change or are added/removed
Expand Down Expand Up @@ -790,6 +790,13 @@ class Masonry extends BaseComponent(HTMLElement) {
}
}

_removeItem(item) {
item.removeAttribute('_removing');
item._masonry = null;

this._onItemRemoved(item);
}

/** @private */
_onItemDisconnected(item) {
// We don't care about transitions if the masonry is not in the body
Expand All @@ -802,20 +809,20 @@ class Masonry extends BaseComponent(HTMLElement) {
return;
}

if (!item.hasAttribute('_removing')) {
if (!item.hasAttribute('_removing') && item.showRemoveTransition) {
// Attach again for remove transition
item.setAttribute('_removing', '');
item._ignoreConnectedCallback = true;
this.appendChild(item);
item._ignoreConnectedCallback = false;
commons.transitionEnd(item, () => {
item.remove();
this._removeItem(item);
});
}
// remove transition completed
else {
item.removeAttribute('_removing');
item._masonry = null;

this._onItemRemoved(item);
this._removeItem(item);
}
}

Expand Down Expand Up @@ -962,6 +969,13 @@ class Masonry extends BaseComponent(HTMLElement) {
item._prevDragPos = null;
}

get observedMessages() {
return {
'coral-masonry-item:_connected': '_onItemConnected',
'coral-masonry-item:_selectedchanged': '_onItemSelectedChanged',
};
}

/**
Registry for masonry layouts.
Expand Down Expand Up @@ -1097,6 +1111,6 @@ class Masonry extends BaseComponent(HTMLElement) {
@property {MasonryItem} detail.selection
The newly selected item(s).
*/
}
});

export default Masonry;
78 changes: 61 additions & 17 deletions coral-component-masonry/src/scripts/MasonryItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import {BaseComponent} from '../../../coral-base-component';
import {DragAction} from '../../../coral-dragaction';
import '../../../coral-component-checkbox';
import quickactions from '../templates/quickactions';
import {transform, commons} from '../../../coral-utils';
import {transform, commons, validate} from '../../../coral-utils';
import {Messenger} from '../../../coral-messenger';
import {Decorator} from '../../../coral-decorator';

const CLASSNAME = '_coral-Masonry-item';

Expand All @@ -25,11 +27,14 @@ const CLASSNAME = '_coral-Masonry-item';
@extends {HTMLElement}
@extends {BaseComponent}
*/
class MasonryItem extends BaseComponent(HTMLElement) {
const MasonryItem = Decorator(class extends BaseComponent(HTMLElement) {
/** @ignore */
constructor() {
super();

// messenger
this._messenger = new Messenger(this);

// Represents ownership (necessary when the item is moved which triggers callbacks)
this._masonry = null;

Expand All @@ -38,6 +43,7 @@ class MasonryItem extends BaseComponent(HTMLElement) {

// Template
this._elements = {};

quickactions.call(this._elements);
}

Expand All @@ -56,6 +62,31 @@ class MasonryItem extends BaseComponent(HTMLElement) {
}
}

/**
Specify while disconnecting the item, should it show transition or not.
This is useful when replacing large items, this result in delay.
@type {Boolean}
@default true
@private No need to update in public document.
*/
get showRemoveTransition() {
return !(this._showRemoveTransition === false);
}

set showRemoveTransition(value) {
this._showRemoveTransition = transform.booleanAttr(value);
}


/**
Specify whether the item is in removing state or not.
@type {Boolean}
*/
get removing() {
return this.hasAttribute('_removing');
}

/**
Whether the item is selected.
Expand All @@ -69,15 +100,19 @@ class MasonryItem extends BaseComponent(HTMLElement) {
}

set selected(value) {
this._selected = transform.booleanAttr(value);
this._reflectAttribute('selected', this._selected);
value = transform.booleanAttr(value);

this.setAttribute('aria-selected', this._selected);
this.classList.toggle('is-selected', this._selected);
if(validate.valueMustChange(this._selected, value)) {
this._selected = value;
this._reflectAttribute('selected', value);

this._elements.check[this._selected ? 'setAttribute' : 'removeAttribute']('checked', '');
this.setAttribute('aria-selected', value);
this.classList.toggle('is-selected', value);

this.trigger('coral-masonry-item:_selectedchanged');
this._elements.check[value ? 'setAttribute' : 'removeAttribute']('checked', '');

this._messenger.postMessage('coral-masonry-item:_selectedchanged');
}
}

/**
Expand Down Expand Up @@ -149,15 +184,27 @@ class MasonryItem extends BaseComponent(HTMLElement) {
}

/** @ignore */
connectedCallback() {
if (this._skipConnectedCallback()) {
return;
_updateCallback(connected) {
super._updateCallback(connected);
if(connected) {
this._messenger.connect();
// In case an already connected element is switched to new parent,
// we need to ignore the connected callback in that case as well which is correct,
// as the item will be connected to new parent and messenger needs to be informed as well parent.
// Hence posting connected in update callback.
this._messenger.postMessage('coral-masonry-item:_connected');
} else {
this._messenger.disconnect();
}
}

/** @ignore */
connectedCallback() {
this._messenger.connect();
super.connectedCallback();

// Inform masonry immediately
this.trigger('coral-masonry-item:_connected');
this._messenger.postMessage('coral-masonry-item:_connected');
}

/** @ignore */
Expand All @@ -181,18 +228,15 @@ class MasonryItem extends BaseComponent(HTMLElement) {

/** @ignore */
disconnectedCallback() {
if (this.isConnected) {
return;
}

super.disconnectedCallback();

// Handle it in masonry immediately
const masonry = this._masonry;
if (masonry) {
masonry._onItemDisconnected(this);
}
this._messenger.disconnect();
}
}
});

export default MasonryItem;

0 comments on commit 40aa149

Please sign in to comment.