From 365512e102c828231f63d295dc58ce536fd98b8b Mon Sep 17 00:00:00 2001 From: Stacey Van Herk Date: Mon, 29 Nov 2021 14:54:29 -0500 Subject: [PATCH 1/4] Add subscription controllers and hookup to the selection components --- components/list/list-item-checkbox-mixin.js | 4 +- components/selection/selection-input.js | 29 ++-- components/selection/selection-mixin.js | 95 +++++------- .../selection/selection-observer-mixin.js | 56 ++----- components/selection/selection-select-all.js | 4 +- components/selection/selection-summary.js | 2 +- helpers/subscriptionControllers.js | 140 ++++++++++++++++++ 7 files changed, 211 insertions(+), 119 deletions(-) create mode 100644 helpers/subscriptionControllers.js diff --git a/components/list/list-item-checkbox-mixin.js b/components/list/list-item-checkbox-mixin.js index 58d3c5b2e9..0cf0c0b420 100644 --- a/components/list/list-item-checkbox-mixin.js +++ b/components/list/list-item-checkbox-mixin.js @@ -181,13 +181,13 @@ export const ListItemCheckboxMixin = superclass => class extends SkeletonMixin(L if (this._selectionProvider === nestedList) return; if (this._selectionProvider && this._selectionProvider !== nestedList) { - this._selectionProvider.unsubscribeObserver(this); + this._selectionProvider.getController('observer').unsubscribe(this); this._selectionProvider = null; } if (nestedList) { this._selectionProvider = nestedList; - this._selectionProvider.subscribeObserver(this); + this._selectionProvider.getController('observer').subscribe(this); } } diff --git a/components/selection/selection-input.js b/components/selection/selection-input.js index 640cf5ef48..d1d0707e83 100644 --- a/components/selection/selection-input.js +++ b/components/selection/selection-input.js @@ -5,6 +5,7 @@ import { ifDefined } from 'lit-html/directives/if-defined.js'; import { LabelledMixin } from '../../mixins/labelled-mixin.js'; import { radioStyles } from '../inputs/input-radio-styles.js'; import { SkeletonMixin } from '../skeleton/skeleton-mixin.js'; +import { SubscriberController } from '../../helpers/subscriptionControllers.js'; const keyCodes = { SPACE: 32 @@ -40,8 +41,7 @@ class Input extends SkeletonMixin(LabelledMixin(LitElement)) { * @type {string} */ key: { type: String }, - _indeterminate: { type: Boolean }, - _provider: { type: Object } + _indeterminate: { type: Boolean } }; } @@ -61,26 +61,21 @@ class Input extends SkeletonMixin(LabelledMixin(LitElement)) { super(); this.selected = false; this._indeterminate = false; + + this._subscriberController = new SubscriberController(this, + { onSubscribe: () => { this.requestUpdate(); } }, + { eventName: 'd2l-selection-input-subscribe', controllerId: 'input' } + ); } connectedCallback() { super.connectedCallback(); - // delay subscription otherwise import/upgrade order can cause selection mixin to miss event - requestAnimationFrame(() => { - const evt = new CustomEvent('d2l-selection-input-subscribe', { - bubbles: true, - composed: true, - detail: {} - }); - this.dispatchEvent(evt); - this._provider = evt.detail.provider; - }); + this._subscriberController.hostConnected(); } disconnectedCallback() { super.disconnectedCallback(); - if (!this._provider) return; - this._provider.unsubscribeSelectable(this); + this._subscriberController.hostDisconnected(); } firstUpdated(changedProperties) { @@ -89,8 +84,8 @@ class Input extends SkeletonMixin(LabelledMixin(LitElement)) { } render() { - if (!this._provider) return; - if (this._provider.selectionSingle) { + if (!this._subscriberController.provider) return; + if (this._subscriberController.provider.selectionSingle) { const radioClasses = { 'd2l-input-radio': true, 'd2l-selection-input-radio': true, @@ -128,6 +123,8 @@ class Input extends SkeletonMixin(LabelledMixin(LitElement)) { updated(changedProperties) { super.updated(changedProperties); + this._subscriberController.hostUpdated(changedProperties); + if ((changedProperties.has('selected') && !(changedProperties.get('selected') === undefined && this.selected === false)) || (changedProperties.has('_indeterminate') && !(changedProperties.get('_indeterminate') === undefined && this._indeterminate === false))) { diff --git a/components/selection/selection-mixin.js b/components/selection/selection-mixin.js index ccc4c4e04c..cecb4d8c72 100644 --- a/components/selection/selection-mixin.js +++ b/components/selection/selection-mixin.js @@ -1,3 +1,4 @@ +import { ProviderController } from '../../helpers/subscriptionControllers.js'; import { RtlMixin } from '../../mixins/rtl-mixin.js'; const keyCodes = { @@ -49,17 +50,27 @@ export const SelectionMixin = superclass => class extends RtlMixin(superclass) { constructor() { super(); this.selectionSingle = false; - this._selectionObservers = new Map(); - this._selectionSelectables = new Map(); + + this._observerController = new ProviderController(this, + { updateSubscribers: this._updateObservers.bind(this) }, + { eventName: 'd2l-selection-observer-subscribe' } + ); + + this._selectablesController = new ProviderController(this, + { onSubscribe: this._subscribeSelectable.bind(this), onUnsubscribe: this._unsubscribeSelectable.bind(this) }, + { eventName: 'd2l-selection-input-subscribe' } + ); + } connectedCallback() { super.connectedCallback(); + if (this._observerController) this._observerController.hostConnected(); + if (this._selectablesController) this._selectablesController.hostConnected(); + if (this.selectionSingle) this.addEventListener('keydown', this._handleRadioKeyDown); if (this.selectionSingle) this.addEventListener('keyup', this._handleRadioKeyUp); this.addEventListener('d2l-selection-change', this._handleSelectionChange); - this.addEventListener('d2l-selection-observer-subscribe', this._handleSelectionObserverSubscribe); - this.addEventListener('d2l-selection-input-subscribe', this._handleSelectionInputSubscribe); requestAnimationFrame(() => { /** @ignore */ this.dispatchEvent(new CustomEvent('d2l-selection-provider-connected', { bubbles: true, composed: true })); @@ -69,24 +80,33 @@ export const SelectionMixin = superclass => class extends RtlMixin(superclass) { disconnectedCallback() { super.disconnectedCallback(); + if (this._observerController) this._observerController.hostDisconnected(); + if (this._selectablesController) this._selectablesController.hostDisconnected(); + if (this.selectionSingle) this.removeEventListener('keydown', this._handleRadioKeyDown); if (this.selectionSingle) this.removeEventListener('keyup', this._handleRadioKeyUp); this.removeEventListener('d2l-selection-change', this._handleSelectionChange); - this.removeEventListener('d2l-selection-observer-subscribe', this._handleSelectionObserverSubscribe); - this.removeEventListener('d2l-selection-input-subscribe', this._handleSelectionInputSubscribe); + } + + getController(controllerId) { + if (controllerId === 'observer') { + return this._observerController; + } else if (controllerId === 'input') { + return this._selectablesController; + } } getSelectionInfo() { let state = SelectionInfo.states.none; const keys = []; - this._selectionSelectables.forEach(selectable => { + this._selectablesController.subscribers.forEach(selectable => { if (selectable.selected) keys.push(selectable.key); if (selectable._indeterminate) state = SelectionInfo.states.some; }); if (keys.length > 0) { - if (keys.length === this._selectionSelectables.size) state = SelectionInfo.states.all; + if (keys.length === this._selectablesController.subscribers.size) state = SelectionInfo.states.all; else state = SelectionInfo.states.some; } @@ -96,27 +116,12 @@ export const SelectionMixin = superclass => class extends RtlMixin(superclass) { setSelectionForAll(selected) { if (this.selectionSingle && selected) return; - this._selectionSelectables.forEach(selectable => { + this._selectablesController.subscribers.forEach(selectable => { if (!!selectable.selected !== selected) { selectable.selected = selected; } }); - this._updateSelectionObservers(); - } - - subscribeObserver(target) { - if (this._selectionObservers.has(target)) return; - this._selectionObservers.set(target, target); - this._updateSelectionObservers(); - } - - unsubscribeObserver(target) { - this._selectionObservers.delete(target); - } - - unsubscribeSelectable(target) { - this._selectionSelectables.delete(target); - this._updateSelectionObservers(); + this._observerController.updateSubscribers(); } _handleRadioKeyDown(e) { @@ -133,7 +138,7 @@ export const SelectionMixin = superclass => class extends RtlMixin(superclass) { if (!e.composedPath()[0].classList.contains('d2l-selection-input-radio')) return; if (e.keyCode < keyCodes.LEFT || e.keyCode > keyCodes.DOWN) return; - const selectables = Array.from(this._selectionSelectables.values()); + const selectables = Array.from(this._selectablesController.subscribers.values()); let currentIndex = selectables.findIndex(selectable => selectable.selected); if (currentIndex === -1) currentIndex = 0; let newIndex; @@ -156,49 +161,31 @@ export const SelectionMixin = superclass => class extends RtlMixin(superclass) { _handleSelectionChange(e) { if (this.selectionSingle && e.detail.selected) { const target = e.composedPath().find(elem => elem.tagName === 'D2L-SELECTION-INPUT'); - this._selectionSelectables.forEach(selectable => { + this._selectablesController.subscribers.forEach(selectable => { if (selectable.selected && selectable !== target) selectable.selected = false; }); } - this._updateSelectionObservers(); + this._observerController.updateSubscribers(); } - _handleSelectionInputSubscribe(e) { - e.stopPropagation(); - e.detail.provider = this; - const target = e.composedPath()[0]; - if (this._selectionSelectables.has(target)) return; - this._selectionSelectables.set(target, target); - + _subscribeSelectable(target) { if (this.selectionSingle && target.selected) { // check invalid usage/state - make sure no others are selected - this._selectionSelectables.forEach(selectable => { + this._selectablesController.subscribers.forEach(selectable => { if (selectable.selected && selectable !== target) selectable.selected = false; }); } - this._updateSelectionObservers(); + this._observerController.updateSubscribers(); } - _handleSelectionObserverSubscribe(e) { - e.stopPropagation(); - e.detail.provider = this; - const target = e.composedPath()[0]; - this.subscribeObserver(target); + _unsubscribeSelectable() { + this._observerController.updateSubscribers(); } - _updateSelectionObservers() { - if (!this._selectionObservers || this._selectionObservers.size === 0) return; - - // debounce the updates for select-all case - if (this._updateObserversRequested) return; - - this._updateObserversRequested = true; - setTimeout(() => { - const info = this.getSelectionInfo(true); - this._selectionObservers.forEach(observer => observer.selectionInfo = info); - this._updateObserversRequested = false; - }, 0); + _updateObservers(observers) { + const info = this.getSelectionInfo(true); + observers.forEach(observer => observer.selectionInfo = info); } }; diff --git a/components/selection/selection-observer-mixin.js b/components/selection/selection-observer-mixin.js index 156d8233ae..65ef8f9bf3 100644 --- a/components/selection/selection-observer-mixin.js +++ b/components/selection/selection-observer-mixin.js @@ -1,5 +1,5 @@ -import { cssEscape } from '../../helpers/dom.js'; import { SelectionInfo } from './selection-mixin.js'; +import { SubscriberController } from '../../helpers/subscriptionControllers.js'; export const SelectionObserverMixin = superclass => class extends superclass { @@ -15,68 +15,36 @@ export const SelectionObserverMixin = superclass => class extends superclass { * @ignore * @type {object} */ - selectionInfo: { type: Object }, - _provider: { type: Object, attribute: false } + selectionInfo: { type: Object } }; } constructor() { super(); this.selectionInfo = new SelectionInfo(); - this._provider = null; + + this._subscriberController = new SubscriberController(this, + { onUnsubscribe: this._clearSelectionInfo.bind(this) }, + { eventName: 'd2l-selection-observer-subscribe', forProperty: 'selectionFor', controllerId: 'observer' } + ); } connectedCallback() { super.connectedCallback(); - // delay subscription otherwise import/upgrade order can cause selection mixin to miss event - requestAnimationFrame(() => { - if (this.selectionFor) return; - - const evt = new CustomEvent('d2l-selection-observer-subscribe', { - bubbles: true, - composed: true, - detail: {} - }); - this.dispatchEvent(evt); - this._provider = evt.detail.provider; - }); + this._subscriberController.hostConnected(); } disconnectedCallback() { super.disconnectedCallback(); - if (this._selectionForObserver) this._selectionForObserver.disconnect(); - if (this._provider) this._provider.unsubscribeObserver(this); + this._subscriberController.hostDisconnected(); } updated(changedProperties) { super.updated(changedProperties); - - if (!changedProperties.has('selectionFor')) return; - - if (this._selectionForObserver) this._selectionForObserver.disconnect(); - if (this._provider) this._provider.unsubscribeObserver(this); - - this._updateProvider(); - - this._selectionForObserver = new MutationObserver(() => { - this._updateProvider(); - }); - - this._selectionForObserver.observe(this.getRootNode(), { - childList: true, - subtree: true - }); + this._subscriberController.hostUpdated(changedProperties); } - _updateProvider() { - const selectionComponent = this.getRootNode().querySelector(`#${cssEscape(this.selectionFor)}`); - if (this._provider === selectionComponent) return; - - this._provider = selectionComponent; - if (this._provider) { - this._provider.subscribeObserver(this); - } else { - this.selectionInfo = new SelectionInfo(); - } + _clearSelectionInfo() { + this.selectionInfo = new SelectionInfo(); } }; diff --git a/components/selection/selection-select-all.js b/components/selection/selection-select-all.js index 17c626d5c6..f62b5c9157 100644 --- a/components/selection/selection-select-all.js +++ b/components/selection/selection-select-all.js @@ -39,7 +39,7 @@ class SelectAll extends LocalizeCoreElement(SelectionObserverMixin(LitElement)) } render() { - if (this._provider && this._provider.selectionSingle) return; + if (this._subscriberController.provider && this._subscriberController.provider.selectionSingle) return; const summary = (this.selectionInfo.state === SelectionInfo.states.none ? this.localize('components.selection.select-all') : this.localize('components.selection.selected', 'count', this.selectionInfo.keys.length)); @@ -62,7 +62,7 @@ class SelectAll extends LocalizeCoreElement(SelectionObserverMixin(LitElement)) } _handleCheckboxChange(e) { - if (this._provider) this._provider.setSelectionForAll(e.target.checked); + if (this._subscriberController.provider) this._subscriberController.provider.setSelectionForAll(e.target.checked); } } diff --git a/components/selection/selection-summary.js b/components/selection/selection-summary.js index d46f7f1aef..00a281524e 100644 --- a/components/selection/selection-summary.js +++ b/components/selection/selection-summary.js @@ -35,7 +35,7 @@ class Summary extends LocalizeCoreElement(SelectionObserverMixin(LitElement)) { } render() { - if (this._provider && this._provider.selectionSingle) return; + if (this._subscriberController.provider && this._subscriberController.provider.selectionSingle) return; const summary = (this.selectionInfo.state === SelectionInfo.states.none && this.noSelectionText ? this.noSelectionText : this.localize('components.selection.selected', 'count', this.selectionInfo.keys.length)); diff --git a/helpers/subscriptionControllers.js b/helpers/subscriptionControllers.js new file mode 100644 index 0000000000..964eccc4ae --- /dev/null +++ b/helpers/subscriptionControllers.js @@ -0,0 +1,140 @@ +import { cssEscape } from '../../helpers/dom.js'; + +export class ProviderController { + + constructor(host, callbacks, options) { + this._host = host; + this._callbacks = callbacks; + this._eventName = options.eventName; + this.subscribers = new Map(); + + this._handleSubscribe = this._handleSubscribe.bind(this); + } + + hostConnected() { + if (this._eventName) this._host.addEventListener(this._eventName, this._handleSubscribe); + } + + hostDisconnected() { + if (this._eventName) this._host.removeEventListener(this._eventName, this._handleSubscribe); + } + + subscribe(target) { + if (this.subscribers.has(target)) return; + this.subscribers.set(target, target); + this.updateSubscribers(); + if (this._callbacks.onSubscribe) this._callbacks.onSubscribe(target); + } + + unsubscribe(target) { + this.subscribers.delete(target); + if (this._callbacks.onUnsubscribe) this._callbacks.onUnsubscribe(target); + } + + updateSubscribers() { + if (!this.subscribers || this.subscribers.size === 0) return; + if (!this._callbacks.updateSubscribers) return; + + // debounce the updates + if (this._updateSubscribersRequested) return; + + this._updateSubscribersRequested = true; + setTimeout(() => { + this._callbacks.updateSubscribers(this.subscribers); + this._updateSubscribersRequested = false; + }, 0); + } + + _handleSubscribe(e) { + e.stopPropagation(); + e.detail.provider = this._host; + const target = e.composedPath()[0]; + this.subscribe(target); + } +} + +export class SubscriberController { + + constructor(host, callbacks, options) { + this._host = host; + this._callbacks = callbacks; + this._eventName = options.eventName; + this._forPropertyName = options.forProperty; + this._controllerId = options.controllerId; + this._providers = new Map(); + } + + // This method is basically offered for convenience + // Components using the event method or a forProperty that takes a string will know they only have one provider + get provider() { + return this._providers.values().next().value; + } + + get providers() { + return Array.from(this._providers.values()); + } + + hostConnected() { + if (!this._eventName || this._host[this._forPropertyName]) return; + // delay subscription otherwise import/upgrade order can cause selection mixin to miss event + requestAnimationFrame(() => { + const evt = new CustomEvent(this._eventName, { + bubbles: true, + composed: true, + detail: {} + }); + this._host.dispatchEvent(evt); + // When using the event method, the provider is not guarenteed to have an id so we use the provider as the key here + this._providers.set(evt.detail.provider, evt.detail.provider); + if (this._callbacks.onSubscribe) this._callbacks.onSubscribe(evt.detail.provider); + }); + } + + hostDisconnected() { + if (this._forObserver) this._forObserver.disconnect(); + this._providers.forEach(provider => { + provider.getController(this._controllerId).unsubscribe(this._host); + }); + } + + hostUpdated(changedProperties) { + if (!changedProperties.has(this._forPropertyName)) return; + + if (this._forObserver) this._forObserver.disconnect(); + this._providers.forEach(provider => { + provider.getController(this._controllerId).unsubscribe(this._host); + }); + this._providers = new Map(); + + this._updateProviders(); + + this._forObserver = new MutationObserver(() => { + this._updateProviders(); + }); + + this._forObserver.observe(this._host.getRootNode(), { + childList: true, + subtree: true + }); + } + + _updateProviders() { + let providerIds = this._host[this._forPropertyName]; + if (typeof(providerIds) === 'string') providerIds = [providerIds]; + + providerIds.forEach(providerId => { + const providerComponent = this._host.getRootNode().querySelector(`#${cssEscape(providerId)}`); + if (this._providers.get(providerId) === providerComponent) return; + + if (providerComponent) { + providerComponent.getController(this._controllerId).subscribe(this._host); + this._providers.set(providerId, providerComponent); + if (this._callbacks.onSubscribe) this._callbacks.onSubscribe(providerComponent); + } else { + this._providers.delete(providerId); + if (this._callbacks.onUnsubscribe) this._callbacks.onUnsubscribe(providerComponent); + } + }); + } + +} From 23075809e7bf9355a4629fd6e2fe0f2b92e7eb4a Mon Sep 17 00:00:00 2001 From: Stacey Van Herk Date: Tue, 30 Nov 2021 16:45:32 -0500 Subject: [PATCH 2/4] Divide the event and for property subscribers into separate controllers --- components/selection/selection-input.js | 6 +-- .../selection/selection-observer-mixin.js | 26 +++++++--- components/selection/selection-select-all.js | 6 ++- components/selection/selection-summary.js | 3 +- helpers/subscriptionControllers.js | 51 ++++++++++++------- 5 files changed, 61 insertions(+), 31 deletions(-) diff --git a/components/selection/selection-input.js b/components/selection/selection-input.js index d1d0707e83..a34cb8d838 100644 --- a/components/selection/selection-input.js +++ b/components/selection/selection-input.js @@ -5,7 +5,7 @@ import { ifDefined } from 'lit-html/directives/if-defined.js'; import { LabelledMixin } from '../../mixins/labelled-mixin.js'; import { radioStyles } from '../inputs/input-radio-styles.js'; import { SkeletonMixin } from '../skeleton/skeleton-mixin.js'; -import { SubscriberController } from '../../helpers/subscriptionControllers.js'; +import { EventSubscriberController } from '../../helpers/subscriptionControllers.js'; const keyCodes = { SPACE: 32 @@ -62,7 +62,7 @@ class Input extends SkeletonMixin(LabelledMixin(LitElement)) { this.selected = false; this._indeterminate = false; - this._subscriberController = new SubscriberController(this, + this._subscriberController = new EventSubscriberController(this, { onSubscribe: () => { this.requestUpdate(); } }, { eventName: 'd2l-selection-input-subscribe', controllerId: 'input' } ); @@ -123,8 +123,6 @@ class Input extends SkeletonMixin(LabelledMixin(LitElement)) { updated(changedProperties) { super.updated(changedProperties); - this._subscriberController.hostUpdated(changedProperties); - if ((changedProperties.has('selected') && !(changedProperties.get('selected') === undefined && this.selected === false)) || (changedProperties.has('_indeterminate') && !(changedProperties.get('_indeterminate') === undefined && this._indeterminate === false))) { diff --git a/components/selection/selection-observer-mixin.js b/components/selection/selection-observer-mixin.js index 65ef8f9bf3..848f216d7c 100644 --- a/components/selection/selection-observer-mixin.js +++ b/components/selection/selection-observer-mixin.js @@ -1,5 +1,5 @@ +import { EventSubscriberController, ForPropertySubscriberController } from '../../helpers/subscriptionControllers.js'; import { SelectionInfo } from './selection-mixin.js'; -import { SubscriberController } from '../../helpers/subscriptionControllers.js'; export const SelectionObserverMixin = superclass => class extends superclass { @@ -23,28 +23,42 @@ export const SelectionObserverMixin = superclass => class extends superclass { super(); this.selectionInfo = new SelectionInfo(); - this._subscriberController = new SubscriberController(this, + this._eventSubscriberController = new EventSubscriberController(this, {}, + { eventName: 'd2l-selection-observer-subscribe', controllerId: 'observer' } + ); + + this._forPropertySubscriberController = new ForPropertySubscriberController(this, { onUnsubscribe: this._clearSelectionInfo.bind(this) }, - { eventName: 'd2l-selection-observer-subscribe', forProperty: 'selectionFor', controllerId: 'observer' } + { forProperty: 'selectionFor', controllerId: 'observer' } ); } connectedCallback() { super.connectedCallback(); - this._subscriberController.hostConnected(); + this._eventSubscriberController.hostConnected(); } disconnectedCallback() { super.disconnectedCallback(); - this._subscriberController.hostDisconnected(); + this._eventSubscriberController.hostDisconnected(); + this._forPropertySubscriberController.hostDisconnected(); } updated(changedProperties) { super.updated(changedProperties); - this._subscriberController.hostUpdated(changedProperties); + this._forPropertySubscriberController.hostUpdated(changedProperties); } _clearSelectionInfo() { this.selectionInfo = new SelectionInfo(); } + + _getSelectionProvider() { + if (this.selectionFor) { + // Selection components currently only support one provider id in selectionFor + return this._forPropertySubscriberController.providers[0]; + } else { + return this._eventSubscriberController.provider; + } + } }; diff --git a/components/selection/selection-select-all.js b/components/selection/selection-select-all.js index f62b5c9157..68d59da4e8 100644 --- a/components/selection/selection-select-all.js +++ b/components/selection/selection-select-all.js @@ -39,7 +39,8 @@ class SelectAll extends LocalizeCoreElement(SelectionObserverMixin(LitElement)) } render() { - if (this._subscriberController.provider && this._subscriberController.provider.selectionSingle) return; + const provider = this._getSelectionProvider(); + if (provider && provider.selectionSingle) return; const summary = (this.selectionInfo.state === SelectionInfo.states.none ? this.localize('components.selection.select-all') : this.localize('components.selection.selected', 'count', this.selectionInfo.keys.length)); @@ -62,7 +63,8 @@ class SelectAll extends LocalizeCoreElement(SelectionObserverMixin(LitElement)) } _handleCheckboxChange(e) { - if (this._subscriberController.provider) this._subscriberController.provider.setSelectionForAll(e.target.checked); + const provider = this._getSelectionProvider(); + if (provider) provider.setSelectionForAll(e.target.checked); } } diff --git a/components/selection/selection-summary.js b/components/selection/selection-summary.js index 00a281524e..889f5bd0ca 100644 --- a/components/selection/selection-summary.js +++ b/components/selection/selection-summary.js @@ -35,7 +35,8 @@ class Summary extends LocalizeCoreElement(SelectionObserverMixin(LitElement)) { } render() { - if (this._subscriberController.provider && this._subscriberController.provider.selectionSingle) return; + const provider = this._getSelectionProvider(); + if (provider && provider.selectionSingle) return; const summary = (this.selectionInfo.state === SelectionInfo.states.none && this.noSelectionText ? this.noSelectionText : this.localize('components.selection.selected', 'count', this.selectionInfo.keys.length)); diff --git a/helpers/subscriptionControllers.js b/helpers/subscriptionControllers.js index 964eccc4ae..301719e435 100644 --- a/helpers/subscriptionControllers.js +++ b/helpers/subscriptionControllers.js @@ -4,8 +4,8 @@ export class ProviderController { constructor(host, callbacks, options) { this._host = host; - this._callbacks = callbacks; - this._eventName = options.eventName; + this._callbacks = callbacks || {}; + this._eventName = options && options.eventName; this.subscribers = new Map(); this._handleSubscribe = this._handleSubscribe.bind(this); @@ -53,29 +53,25 @@ export class ProviderController { } } -export class SubscriberController { +export class EventSubscriberController { constructor(host, callbacks, options) { this._host = host; - this._callbacks = callbacks; - this._eventName = options.eventName; - this._forPropertyName = options.forProperty; - this._controllerId = options.controllerId; - this._providers = new Map(); + this._callbacks = callbacks || {}; + this._eventName = options && options.eventName; + this._controllerId = options && options.controllerId; + this._provider = null; } - // This method is basically offered for convenience - // Components using the event method or a forProperty that takes a string will know they only have one provider get provider() { - return this._providers.values().next().value; - } - - get providers() { - return Array.from(this._providers.values()); + return this._provider; } hostConnected() { - if (!this._eventName || this._host[this._forPropertyName]) return; + /* Do we try not to fire this event if someone wants to use the other provider instead based on set attributes? + * That's really the selection-observer-mixin's problem, not these cotnrollers, who should assume you want to use them... + */ + // delay subscription otherwise import/upgrade order can cause selection mixin to miss event requestAnimationFrame(() => { const evt = new CustomEvent(this._eventName, { @@ -84,12 +80,31 @@ export class SubscriberController { detail: {} }); this._host.dispatchEvent(evt); - // When using the event method, the provider is not guarenteed to have an id so we use the provider as the key here - this._providers.set(evt.detail.provider, evt.detail.provider); + this._provider = evt.detail.provider; if (this._callbacks.onSubscribe) this._callbacks.onSubscribe(evt.detail.provider); }); } + hostDisconnected() { + if (this._provider) this._provider.getController(this._controllerId).unsubscribe(this._host); + } + +} + +export class ForPropertySubscriberController { + + constructor(host, callbacks, options) { + this._host = host; + this._callbacks = callbacks || {}; + this._forPropertyName = options && options.forProperty; + this._controllerId = options && options.controllerId; + this._providers = new Map(); + } + + get providers() { + return Array.from(this._providers.values()); + } + hostDisconnected() { if (this._forObserver) this._forObserver.disconnect(); this._providers.forEach(provider => { From 015aec7f386780019d1351216142bdc42f4a39fc Mon Sep 17 00:00:00 2001 From: Stacey Van Herk Date: Wed, 1 Dec 2021 17:40:43 -0500 Subject: [PATCH 3/4] Add form and validation POC --- components/form/form-element-mixin.js | 44 +++++++------- components/form/form-mixin.js | 27 ++++----- components/selection/selection-input.js | 3 +- .../validation/validation-custom-mixin.js | 57 ++++++++----------- 4 files changed, 56 insertions(+), 75 deletions(-) diff --git a/components/form/form-element-mixin.js b/components/form/form-element-mixin.js index 758ea61984..57400abc8b 100644 --- a/components/form/form-element-mixin.js +++ b/components/form/form-element-mixin.js @@ -1,5 +1,6 @@ import { isCustomFormElement } from './form-helper.js'; import { LocalizeCoreElement } from '../../lang/localize-core-element.js'; +import { ProviderController } from '../../helpers/subscriptionControllers.js'; export class FormElementValidityState { @@ -111,10 +112,8 @@ export const FormElementMixin = superclass => class extends LocalizeCoreElement( constructor() { super(); - this._validationCustomConnected = this._validationCustomConnected.bind(this); this._onFormElementErrorsChange = this._onFormElementErrorsChange.bind(this); - this._validationCustoms = new Set(); this._validity = new FormElementValidityState({}); /** @ignore */ this.forceInvalid = false; @@ -130,7 +129,10 @@ export const FormElementMixin = superclass => class extends LocalizeCoreElement( this.childErrors = new Map(); this._errors = []; - this.shadowRoot.addEventListener('d2l-validation-custom-connected', this._validationCustomConnected); + this._validationCustomsController = new ProviderController(this, {}, + { eventName: 'd2l-validation-custom-connected' } + ); + this.shadowRoot.addEventListener('d2l-form-element-errors-change', this._onFormElementErrorsChange); } @@ -153,6 +155,16 @@ export const FormElementMixin = superclass => class extends LocalizeCoreElement( return this._validity; } + connectedCallback() { + super.connectedCallback(); + this._validationCustomsController.hostConnected(); + } + + disconnectedCallback() { + super.disconnectedCallback(); + this._validationCustomsController.hostDisconnected(); + } + updated(changedProperties) { if (changedProperties.has('_errors') || changedProperties.has('childErrors')) { let errors = this._errors; @@ -173,11 +185,15 @@ export const FormElementMixin = superclass => class extends LocalizeCoreElement( } } + getController() { + return this._validationCustomsController; + } + async requestValidate(showNewErrors = true) { if (this.noValidate) { return []; } - const customs = [...this._validationCustoms].filter(custom => custom.forElement === this || !isCustomFormElement(custom.forElement)); + const customs = Array.from(this._validationCustomsController.subscribers.values()).filter(custom => custom.forElement === this || !isCustomFormElement(custom.forElement)); const results = await Promise.all(customs.map(custom => custom.validate())); const errors = customs.map(custom => custom.failureText).filter((_, i) => !results[i]); if (!this.validity.valid) { @@ -208,14 +224,6 @@ export const FormElementMixin = superclass => class extends LocalizeCoreElement( return this._errors; } - validationCustomConnected(custom) { - this._validationCustoms.add(custom); - } - - validationCustomDisconnected(custom) { - this._validationCustoms.delete(custom); - } - _onFormElementErrorsChange(e) { e.stopPropagation(); const errors = e.detail.errors; @@ -230,16 +238,4 @@ export const FormElementMixin = superclass => class extends LocalizeCoreElement( } } - _validationCustomConnected(e) { - e.stopPropagation(); - const custom = e.composedPath()[0]; - this.validationCustomConnected(custom); - - const onDisconnect = () => { - custom.removeEventListener('d2l-validation-custom-disconnected', onDisconnect); - this.validationCustomDisconnected(custom); - }; - custom.addEventListener('d2l-validation-custom-disconnected', onDisconnect); - } - }; diff --git a/components/form/form-mixin.js b/components/form/form-mixin.js index 98afbc8a54..e075cfc75d 100644 --- a/components/form/form-mixin.js +++ b/components/form/form-mixin.js @@ -6,6 +6,7 @@ import { getComposedActiveElement } from '../../helpers/focus.js'; import { getUniqueId } from '../../helpers/uniqueId.js'; import { LocalizeCoreElement } from '../../lang/localize-core-element.js'; import { localizeFormElement } from './form-element-localize-helper.js'; +import { ProviderController } from '../../helpers/subscriptionControllers.js'; export const FormMixin = superclass => class extends LocalizeCoreElement(superclass) { @@ -27,21 +28,25 @@ export const FormMixin = superclass => class extends LocalizeCoreElement(supercl this.trackChanges = false; this._tooltips = new Map(); - this._validationCustoms = new Set(); this._errors = new Map(); + this._validationCustomsController = new ProviderController(this, {}, + { eventName: 'd2l-validation-custom-connected' } + ); + this.addEventListener('d2l-form-errors-change', this._onErrorsChange); this.addEventListener('d2l-form-element-errors-change', this._onErrorsChange); - this.addEventListener('d2l-validation-custom-connected', this._validationCustomConnected); } connectedCallback() { super.connectedCallback(); + this._validationCustomsController.hostConnected(); window.addEventListener('beforeunload', this._onUnload); } disconnectedCallback() { super.disconnectedCallback(); + this._validationCustomsController.hostDisconnected(); window.removeEventListener('beforeunload', this._onUnload); } @@ -52,6 +57,10 @@ export const FormMixin = superclass => class extends LocalizeCoreElement(supercl this.addEventListener('focusout', this._onFormElementChange); } + getController() { + return this._validationCustomsController; + } + // eslint-disable-next-line no-unused-vars async requestSubmit(submitter) { throw new Error('FormMixin.requestSubmit must be overridden'); @@ -154,7 +163,7 @@ export const FormMixin = superclass => class extends LocalizeCoreElement(supercl if (isCustomFormElement(ele)) { return ele.validate(showNewErrors); } else if (isNativeFormElement(ele)) { - const customs = [...this._validationCustoms].filter(custom => custom.forElement === ele); + const customs = Array.from(this._validationCustomsController.subscribers.values()).filter(custom => custom.forElement === ele); const results = await Promise.all(customs.map(custom => custom.validate())); const errors = customs.map(custom => custom.failureText).filter((_, i) => !results[i]); if (!ele.checkValidity()) { @@ -171,16 +180,4 @@ export const FormMixin = superclass => class extends LocalizeCoreElement(supercl return []; } - _validationCustomConnected(e) { - e.stopPropagation(); - const custom = e.composedPath()[0]; - this._validationCustoms.add(custom); - - const onDisconnect = () => { - custom.removeEventListener('d2l-validation-custom-disconnected', onDisconnect); - this._validationCustoms.delete(custom); - }; - custom.addEventListener('d2l-validation-custom-disconnected', onDisconnect); - } - }; diff --git a/components/selection/selection-input.js b/components/selection/selection-input.js index a34cb8d838..2159b2cf6b 100644 --- a/components/selection/selection-input.js +++ b/components/selection/selection-input.js @@ -1,12 +1,11 @@ import '../inputs/input-checkbox.js'; import { css, html, LitElement } from 'lit-element/lit-element.js'; import { classMap } from 'lit-html/directives/class-map.js'; +import { EventSubscriberController } from '../../helpers/subscriptionControllers.js'; import { ifDefined } from 'lit-html/directives/if-defined.js'; import { LabelledMixin } from '../../mixins/labelled-mixin.js'; import { radioStyles } from '../inputs/input-radio-styles.js'; import { SkeletonMixin } from '../skeleton/skeleton-mixin.js'; -import { EventSubscriberController } from '../../helpers/subscriptionControllers.js'; - const keyCodes = { SPACE: 32 }; diff --git a/components/validation/validation-custom-mixin.js b/components/validation/validation-custom-mixin.js index f7dfa659d6..38a139a4b0 100644 --- a/components/validation/validation-custom-mixin.js +++ b/components/validation/validation-custom-mixin.js @@ -1,4 +1,4 @@ -import { isCustomFormElement } from '../form/form-helper.js'; +import { EventSubscriberController, ForPropertySubscriberController } from '../../helpers/subscriptionControllers.js'; export const ValidationCustomMixin = superclass => class extends superclass { @@ -11,60 +11,49 @@ export const ValidationCustomMixin = superclass => class extends superclass { constructor() { super(); - this._forElement = null; + + this._eventSubscriberController = new EventSubscriberController(this, {}, + { eventName: 'd2l-validation-custom-connected' } + ); + + this._forPropertySubscriberController = new ForPropertySubscriberController(this, + { onUnsubscribe: this._onUnsubscribe.bind(this) }, + { forProperty: 'for' } + ); } get forElement() { - return this._forElement; + if (this.for) { + // Validation custom components only support one for element + return this._forPropertySubscriberController.providers[0]; + } else { + return this._eventSubscriberController.provider; + } } connectedCallback() { super.connectedCallback(); - this._updateForElement(); - this.dispatchEvent(new CustomEvent('d2l-validation-custom-connected', { bubbles: true })); + this._eventSubscriberController.hostConnected(); } disconnectedCallback() { super.disconnectedCallback(); - if (isCustomFormElement(this._forElement)) { - this._forElement.validationCustomDisconnected(this); - } - this._forElement = null; - this.dispatchEvent(new CustomEvent('d2l-validation-custom-disconnected')); + this._eventSubscriberController.hostDisconnected(); + this._forPropertySubscriberController.hostDisconnected(); } updated(changedProperties) { super.updated(changedProperties); - - changedProperties.forEach((_, prop) => { - if (prop === 'for') { - this._updateForElement(); - } - }); + this._forPropertySubscriberController.hostUpdated(changedProperties); } async validate() { throw new Error('ValidationCustomMixin requires validate to be overridden'); } - _updateForElement() { - const oldForElement = this._forElement; - if (this.for) { - const root = this.getRootNode(); - this._forElement = root.getElementById(this.for); - if (!this._forElement) { - throw new Error(`validation-custom failed to find element with id ${this.for}`); - } - } else { - this._forElement = null; - } - if (this._forElement !== oldForElement) { - if (isCustomFormElement(oldForElement)) { - oldForElement.validationCustomDisconnected(this); - } - if (isCustomFormElement(this._forElement)) { - this._forElement.validationCustomConnected(this); - } + _onUnsubscribe() { + if (this._forPropertySubscriberController.provider.length === 0) { + throw new Error(`validation-custom failed to find element with id ${this.for}`); } } From 367dda86bc122642e98cef3264d964f98cbe01a2 Mon Sep 17 00:00:00 2001 From: Stacey Van Herk Date: Tue, 7 Dec 2021 13:12:08 -0500 Subject: [PATCH 4/4] Add fix and error control (to be tested) --- .../validation/validation-custom-mixin.js | 2 +- helpers/subscriptionControllers.js | 41 ++++++++++++++----- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/components/validation/validation-custom-mixin.js b/components/validation/validation-custom-mixin.js index 38a139a4b0..489f6b4c45 100644 --- a/components/validation/validation-custom-mixin.js +++ b/components/validation/validation-custom-mixin.js @@ -25,7 +25,7 @@ export const ValidationCustomMixin = superclass => class extends superclass { get forElement() { if (this.for) { // Validation custom components only support one for element - return this._forPropertySubscriberController.providers[0]; + return this._forPropertySubscriberController.providers.length > 0 ? this._forPropertySubscriberController.providers[0] : null; } else { return this._eventSubscriberController.provider; } diff --git a/helpers/subscriptionControllers.js b/helpers/subscriptionControllers.js index 301719e435..cebef5886a 100644 --- a/helpers/subscriptionControllers.js +++ b/helpers/subscriptionControllers.js @@ -133,22 +133,41 @@ export class ForPropertySubscriberController { }); } + _updateProvider(providerId, elapsedTime) { + const providerComponent = this._host.getRootNode().querySelector(`#${cssEscape(providerId)}`); + if (!providerComponent && this._callbacks.onError) { + if (elapsedTime < 3000) { + setTimeout(() => { + this._updateProvider(providerId, elapsedTime + 100); + }, 100); + } else { + this._callbacks.onError(providerId); + } + } + + if (this._providers.get(providerId) === providerComponent) return; + + if (providerComponent) { + providerComponent.getController(this._controllerId).subscribe(this._host); + this._providers.set(providerId, providerComponent); + if (this._callbacks.onSubscribe) this._callbacks.onSubscribe(providerComponent); + } else { + this._providers.delete(providerId); + if (this._callbacks.onUnsubscribe) this._callbacks.onUnsubscribe(providerComponent); + } + } + _updateProviders() { let providerIds = this._host[this._forPropertyName]; + if (!providerIds) { + // callback for no provider ids? + return; + } + if (typeof(providerIds) === 'string') providerIds = [providerIds]; providerIds.forEach(providerId => { - const providerComponent = this._host.getRootNode().querySelector(`#${cssEscape(providerId)}`); - if (this._providers.get(providerId) === providerComponent) return; - - if (providerComponent) { - providerComponent.getController(this._controllerId).subscribe(this._host); - this._providers.set(providerId, providerComponent); - if (this._callbacks.onSubscribe) this._callbacks.onSubscribe(providerComponent); - } else { - this._providers.delete(providerId); - if (this._callbacks.onUnsubscribe) this._callbacks.onUnsubscribe(providerComponent); - } + this._updateProvider(providerId, 0); }); }