Skip to content

Commit

Permalink
refactor: move controllers logic to ControllerMixin (#3069) (#3075)
Browse files Browse the repository at this point in the history
Co-authored-by: Sergey Vinogradov <[email protected]>
  • Loading branch information
web-padawan and vursen authored Nov 18, 2021
1 parent db354d9 commit 9222a72
Show file tree
Hide file tree
Showing 42 changed files with 314 additions and 162 deletions.
3 changes: 2 additions & 1 deletion packages/checkbox/src/vaadin-checkbox.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { ActiveMixin } from '@vaadin/component-base/src/active-mixin.js';
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
import { CheckedMixin } from '@vaadin/field-base/src/checked-mixin.js';
import { DelegateFocusMixin } from '@vaadin/field-base/src/delegate-focus-mixin.js';
Expand Down Expand Up @@ -61,7 +62,7 @@ export interface CheckboxEventMap extends HTMLElementEventMap, CheckboxCustomEve
* @fires {CustomEvent} indeterminate-changed - Fired when the `indeterminate` property changes.
*/
declare class Checkbox extends SlotLabelMixin(
CheckedMixin(DelegateFocusMixin(ActiveMixin(ElementMixin(ThemableMixin(HTMLElement)))))
CheckedMixin(DelegateFocusMixin(ActiveMixin(ElementMixin(ThemableMixin(ControllerMixin(HTMLElement))))))
) {
/**
* True if the checkbox is in the indeterminate state which means
Expand Down
4 changes: 3 additions & 1 deletion packages/checkbox/src/vaadin-checkbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
import { ActiveMixin } from '@vaadin/component-base/src/active-mixin.js';
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
import { AriaLabelController } from '@vaadin/field-base/src/aria-label-controller.js';
import { CheckedMixin } from '@vaadin/field-base/src/checked-mixin.js';
Expand Down Expand Up @@ -46,6 +47,7 @@ import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mix
* @fires {CustomEvent} indeterminate-changed - Fired when the `indeterminate` property changes.
*
* @extends HTMLElement
* @mixes ControllerMixin
* @mixes ThemableMixin
* @mixes ElementMixin
* @mixes ActiveMixin
Expand All @@ -54,7 +56,7 @@ import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mix
* @mixes SlotLabelMixin
*/
class Checkbox extends SlotLabelMixin(
CheckedMixin(DelegateFocusMixin(ActiveMixin(ElementMixin(ThemableMixin(PolymerElement)))))
CheckedMixin(DelegateFocusMixin(ActiveMixin(ElementMixin(ThemableMixin(ControllerMixin(PolymerElement))))))
) {
static get is() {
return 'vaadin-checkbox';
Expand Down
2 changes: 2 additions & 0 deletions packages/checkbox/test/typings/checkbox.types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import '../../vaadin-checkbox.js';
import { ActiveMixinClass } from '@vaadin/component-base/src/active-mixin.js';
import { ControllerMixinClass } from '@vaadin/component-base/src/controller-mixin.js';
import { DisabledMixinClass } from '@vaadin/component-base/src/disabled-mixin.js';
import { ElementMixinClass } from '@vaadin/component-base/src/element-mixin.js';
import { FocusMixinClass } from '@vaadin/component-base/src/focus-mixin.js';
Expand All @@ -24,6 +25,7 @@ assertType<string>(checkbox.name);
assertType<string>(checkbox.value);

// Mixins
assertType<ControllerMixinClass>(checkbox);
assertType<ActiveMixinClass>(checkbox);
assertType<DisabledMixinClass>(checkbox);
assertType<ElementMixinClass>(checkbox);
Expand Down
4 changes: 3 additions & 1 deletion packages/combo-box/src/vaadin-combo-box.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Copyright (c) 2021 Vaadin Ltd.
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { ControllerMixinClass } from '@vaadin/component-base/src/controller-mixin.js';
import { ElementMixinClass } from '@vaadin/component-base/src/element-mixin.js';
import { InputControlMixinClass } from '@vaadin/field-base/src/input-control-mixin.js';
import { PatternMixinClass } from '@vaadin/field-base/src/pattern-mixin.js';
Expand Down Expand Up @@ -208,7 +209,8 @@ interface ComboBox<TItem = ComboBoxDefaultItem>
PatternMixinClass,
InputControlMixinClass,
ThemableMixinClass,
ElementMixinClass {}
ElementMixinClass,
ControllerMixinClass {}

declare global {
interface HTMLElementTagNameMap {
Expand Down
4 changes: 3 additions & 1 deletion packages/combo-box/src/vaadin-combo-box.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import '@vaadin/input-container/src/vaadin-input-container.js';
import './vaadin-combo-box-dropdown.js';
import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
import { AriaLabelController } from '@vaadin/field-base/src/aria-label-controller.js';
import { InputControlMixin } from '@vaadin/field-base/src/input-control-mixin.js';
Expand Down Expand Up @@ -146,6 +147,7 @@ registerStyles('vaadin-combo-box', inputFieldShared, { moduleId: 'vaadin-combo-b
* @fires {CustomEvent} value-changed - Fired when the `value` property changes.
*
* @extends HTMLElement
* @mixes ControllerMixin
* @mixes ElementMixin
* @mixes ThemableMixin
* @mixes InputControlMixin
Expand All @@ -154,7 +156,7 @@ registerStyles('vaadin-combo-box', inputFieldShared, { moduleId: 'vaadin-combo-b
* @mixes ComboBoxMixin
*/
class ComboBox extends ComboBoxDataProviderMixin(
ComboBoxMixin(PatternMixin(InputControlMixin(ThemableMixin(ElementMixin(PolymerElement)))))
ComboBoxMixin(PatternMixin(InputControlMixin(ThemableMixin(ElementMixin(ControllerMixin(PolymerElement))))))
) {
static get is() {
return 'vaadin-combo-box';
Expand Down
2 changes: 2 additions & 0 deletions packages/combo-box/test/typings/combo-box.types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { ControllerMixinClass } from '@vaadin/component-base/src/controller-mixin.js';
import { ElementMixinClass } from '@vaadin/component-base/src/element-mixin.js';
import { ThemableMixinClass } from '@vaadin/vaadin-themable-mixin';
import { ComboBoxDataProviderMixinClass } from '../../src/vaadin-combo-box-data-provider-mixin';
Expand Down Expand Up @@ -32,6 +33,7 @@ const genericComboBox = document.createElement('vaadin-combo-box');

const narrowedComboBox = genericComboBox as ComboBox<TestComboBoxItem>;
assertType<ComboBox>(narrowedComboBox);
assertType<ControllerMixinClass>(narrowedComboBox);
assertType<ElementMixinClass>(narrowedComboBox);
assertType<ComboBoxDataProviderMixinClass<TestComboBoxItem>>(narrowedComboBox);
assertType<ComboBoxMixinClass<TestComboBoxItem>>(narrowedComboBox);
Expand Down
1 change: 1 addition & 0 deletions packages/component-base/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { ActiveMixin } from './src/active-mixin.js';
export { ControllerMixin } from './src/controller-mixin.js';
export { DirMixin } from './src/dir-mixin.js';
export { DisabledMixin } from './src/disabled-mixin.js';
export { ElementMixin } from './src/element-mixin.js';
Expand Down
1 change: 1 addition & 0 deletions packages/component-base/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { ActiveMixin } from './src/active-mixin.js';
export { ControllerMixin } from './src/controller-mixin.js';
export { DirMixin } from './src/dir-mixin.js';
export { DisabledMixin } from './src/disabled-mixin.js';
export { ElementMixin } from './src/element-mixin.js';
Expand Down
28 changes: 28 additions & 0 deletions packages/component-base/src/controller-mixin.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* @license
* Copyright (c) 2021 Vaadin Ltd.
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { Constructor } from '@open-wc/dedupe-mixin';
import { ReactiveController, ReactiveControllerHost } from 'lit';

/**
* A mixin for connecting controllers to the element.
*/
export declare function ControllerMixin<T extends Constructor<HTMLElement>>(
superclass: T
): T & Constructor<ControllerMixinClass>;

export declare class ControllerMixinClass
implements Pick<ReactiveControllerHost, 'addController' | 'removeController'>
{
/**
* Registers a controller to participate in the element update cycle.
*/
addController(controller: ReactiveController): void;

/**
* Removes a controller from the element.
*/
removeController(controller: ReactiveController): void;
}
67 changes: 67 additions & 0 deletions packages/component-base/src/controller-mixin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* @license
* Copyright (c) 2021 Vaadin Ltd.
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { dedupingMixin } from '@polymer/polymer/lib/utils/mixin.js';

/**
* A mixin for connecting controllers to the element.
*
* @polymerMixin
*/
export const ControllerMixin = dedupingMixin(
(superClass) =>
class ControllerMixinClass extends superClass {
constructor() {
super();

/**
* @type {Set<import('lit').ReactiveController>}
*/
this.__controllers = new Set();
}

/** @protected */
connectedCallback() {
super.connectedCallback();

this.__controllers.forEach((c) => {
c.hostConnected && c.hostConnected();
});
}

/** @protected */
disconnectedCallback() {
super.disconnectedCallback();

this.__controllers.forEach((c) => {
c.hostDisconnected && c.hostDisconnected();
});
}

/**
* Registers a controller to participate in the element update cycle.
*
* @param {import('lit').ReactiveController} controller
* @protected
*/
addController(controller) {
this.__controllers.add(controller);
// Call hostConnected if a controller is added after the element is attached.
if (this.$ !== undefined && this.isConnected && controller.hostConnected) {
controller.hostConnected();
}
}

/**
* Removes a controller from the element.
*
* @param {import('lit').ReactiveController} controller
* @protected
*/
removeController(controller) {
this.__controllers.delete(controller);
}
}
);
8 changes: 2 additions & 6 deletions packages/component-base/src/element-mixin.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@
import '../custom_typings/vaadin-usage-statistics.js';
import '../custom_typings/vaadin.js';
import { Constructor } from '@open-wc/dedupe-mixin';
import { ReactiveControllerHost } from 'lit';
import { DirMixinClass } from './dir-mixin.js';

/**
* A mixin to provide common logic for Vaadin components.
* A mixin providing common logic for Vaadin components.
*/
export declare function ElementMixin<T extends Constructor<HTMLElement>>(
superclass: T
): T &
Constructor<DirMixinClass> &
Constructor<ElementMixinClass> &
Pick<ReactiveControllerHost, 'addController' | 'removeController'>;
): T & Constructor<DirMixinClass> & Constructor<ElementMixinClass>;

export declare class ElementMixinClass {
static version: string;
Expand Down
40 changes: 0 additions & 40 deletions packages/component-base/src/element-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,50 +58,10 @@ export const ElementMixin = (superClass) =>
constructor() {
super();

this.__controllers = new Set();

if (document.doctype === null) {
console.warn(
'Vaadin components require the "standards mode" declaration. Please add <!DOCTYPE html> to the HTML document.'
);
}
}

/** @protected */
connectedCallback() {
super.connectedCallback();

this.__controllers.forEach((c) => {
c.hostConnected && c.hostConnected();
});
}

/** @protected */
disconnectedCallback() {
super.disconnectedCallback();

this.__controllers.forEach((c) => {
c.hostDisconnected && c.hostDisconnected();
});
}

/**
* Registers a controller to participate in the element update cycle.
* @protected
*/
addController(controller) {
this.__controllers.add(controller);
// Call hostConnected if a controller is added after the element is attached.
if (this.$ !== undefined && this.isConnected && controller.hostConnected) {
controller.hostConnected();
}
}

/**
* Removes a controller from the element.
* @protected
*/
removeController(controller) {
this.__controllers.delete(controller);
}
};
82 changes: 82 additions & 0 deletions packages/component-base/test/controller-mixin.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { expect } from '@esm-bundle/chai';
import sinon from 'sinon';
import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
import { ControllerMixin } from '../src/controller-mixin.js';

class ControllerHost extends ControllerMixin(PolymerElement) {
static get is() {
return 'controller-host';
}

static get template() {
return html`Content`;
}
}
customElements.define(ControllerHost.is, ControllerHost);

class SpyController {
constructor(host) {
this.host = host;
}

hostConnected() {
// Used in spy
}

hostDisconnected() {
// Used in spy
}
}

describe('controller-mixin', () => {
let element, controller;

beforeEach(() => {
element = document.createElement('controller-host');
controller = new SpyController(element);
sinon.stub(controller, 'hostConnected');
sinon.stub(controller, 'hostDisconnected');
});

describe('hostConnected', () => {
afterEach(() => {
document.body.removeChild(element);
});

it('should run hostConnected when controller is added before host is attached', () => {
element.addController(controller);
document.body.appendChild(element);
expect(controller.hostConnected.calledOnce).to.be.true;
});

it('should run hostConnected when controller is added after host is attached', () => {
document.body.appendChild(element);
element.addController(controller);
expect(controller.hostConnected.calledOnce).to.be.true;
});
});

describe('hostDisconnected', () => {
beforeEach(() => {
document.body.appendChild(element);
element.addController(controller);
});

it('should run hostDisconnected when host is detached', () => {
document.body.removeChild(element);
expect(controller.hostDisconnected.calledOnce).to.be.true;
});

it('should not run hostDisconnected if controller is removed', () => {
element.removeController(controller);
document.body.removeChild(element);
expect(controller.hostDisconnected.calledOnce).to.be.false;
});

it('should run hostConnected when host is reattached', () => {
document.body.removeChild(element);
document.body.appendChild(element);
expect(controller.hostConnected.calledTwice).to.be.true;
});
});
});
Loading

0 comments on commit 9222a72

Please sign in to comment.