Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

POC - Adding subscription controllers #1901

Closed
wants to merge 4 commits into from
Closed

POC - Adding subscription controllers #1901

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
365512e
Add subscription controllers and hookup to the selection components
svanherk Nov 29, 2021
2307580
Divide the event and for property subscribers into separate controllers
svanherk Nov 30, 2021
015aec7
Add form and validation POC
svanherk Dec 1, 2021
367dda8
Add fix and error control (to be tested)
svanherk Dec 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions components/list/list-item-checkbox-mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
29 changes: 13 additions & 16 deletions components/selection/selection-input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -40,8 +41,7 @@ class Input extends SkeletonMixin(LabelledMixin(LitElement)) {
* @type {string}
*/
key: { type: String },
_indeterminate: { type: Boolean },
_provider: { type: Object }
_indeterminate: { type: Boolean }
};
}

Expand All @@ -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) {
Expand All @@ -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,
Expand Down Expand Up @@ -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))) {

Expand Down
95 changes: 41 additions & 54 deletions components/selection/selection-mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ProviderController } from '../../helpers/subscriptionControllers.js';
import { RtlMixin } from '../../mixins/rtl-mixin.js';

const keyCodes = {
Expand Down Expand Up @@ -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 }));
Expand All @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this, but I couldn't think of another great way to do both:

  1. Expose the controller without a forced naming convention AND
  2. Have the ability for there to be multiple provider controllers in a single component

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;
}

Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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);
}

};
56 changes: 12 additions & 44 deletions components/selection/selection-observer-mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {

Expand All @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good sign -- once we go to Lit 2, connectedCallback, disconnectedCallback and updated can be removed.

}

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();
}
};
4 changes: 2 additions & 2 deletions components/selection/selection-select-all.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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);
}

}
Expand Down
2 changes: 1 addition & 1 deletion components/selection/selection-summary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Loading