diff --git a/components/inputs/input-color.js b/components/inputs/input-color.js index eb166f7d5a2..f9e38355a2f 100644 --- a/components/inputs/input-color.js +++ b/components/inputs/input-color.js @@ -11,6 +11,7 @@ import { getValidHexColor } from '../../helpers/color.js'; import { ifDefined } from 'lit/directives/if-defined.js'; import { inputLabelStyles } from './input-label-styles.js'; import { LocalizeCoreElement } from '../../helpers/localize-core-element.js'; +import { PropertyRequiredMixin } from '../../mixins/property-required/property-required-mixin.js'; import { styleMap } from 'lit/directives/style-map.js'; const DEFAULT_VALUE = '#000000'; @@ -80,7 +81,7 @@ const SWATCH_TRANSPARENT = ` this.type !== 'custom' || hasValue + }); } get associatedValue() { return this._associatedValue; } @@ -258,11 +262,6 @@ class InputColor extends FocusMixin(FormElementMixin(LocalizeCoreElement(LitElem return '#opener'; } - firstUpdated(changedProperties) { - super.firstUpdated(changedProperties); - this._validateLabel(); - } - render() { const label = !this.labelHidden ? html`
${this._getLabel()}
` : nothing; @@ -277,8 +276,6 @@ class InputColor extends FocusMixin(FormElementMixin(LocalizeCoreElement(LitElem super.updated(changedProperties); - if (changedProperties.has('label') || changedProperties.has('type')) this._validateLabel(); - if (changedProperties.has('value') || changedProperties.has('type') || changedProperties.has('disallowNone')) { this.setFormValue(this.value); } @@ -397,17 +394,5 @@ class InputColor extends FocusMixin(FormElementMixin(LocalizeCoreElement(LitElem )); } - _validateLabel() { - clearTimeout(this._validatingLabelTimeout); - // don't error immediately in case it doesn't get set immediately - this._validatingLabelTimeout = setTimeout(() => { - this._validatingLabelTimeout = null; - const hasLabel = (typeof this.label === 'string') && this.label.length > 0; - if (!hasLabel && this.type === 'custom') { - throw new Error(': "label" attribute is required when "type" is "custom"'); - } - }, 3000); - } - } customElements.define('d2l-input-color', InputColor); diff --git a/components/inputs/test/input-color.test.js b/components/inputs/test/input-color.test.js index d7e791b512c..5bc58906628 100644 --- a/components/inputs/test/input-color.test.js +++ b/components/inputs/test/input-color.test.js @@ -1,5 +1,6 @@ import '../input-color.js'; import { expect, fixture, html, oneEvent, runConstructor } from '@brightspace-ui/testing'; +import { createDefaultMessage } from '../../../mixins/property-required/property-required-mixin.js'; describe('d2l-input-color', () => { @@ -128,4 +129,35 @@ describe('d2l-input-color', () => { }); + describe('validation', () => { + + ['foreground', 'background'].forEach(type => { + it(`should not require a label when type is "${type}"`, async() => { + const elem = await fixture(html``); + expect(() => elem.flushRequiredPropertyErrors()).to.not.throw(); + }); + }); + + it('should throw when type is "custom" and no label', async() => { + const elem = await fixture(html``); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage('d2l-input-color', 'label')); + }); + + it('should not throw when type is "custom" and label is provided', async() => { + const elem = await fixture(html``); + expect(() => elem.flushRequiredPropertyErrors()).to.not.throw(); + }); + + it('should require a label when type changes to "custom"', async() => { + const elem = await fixture(html``); + expect(() => elem.flushRequiredPropertyErrors()).to.not.throw(); + elem.setAttribute('type', 'custom'); + await elem.updateComplete; + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage('d2l-input-color', 'label')); + }); + + }); + }); diff --git a/mixins/attribute-required/attribute-required-mixin.js b/mixins/attribute-required/attribute-required-mixin.js deleted file mode 100644 index 1a062087a15..00000000000 --- a/mixins/attribute-required/attribute-required-mixin.js +++ /dev/null @@ -1,54 +0,0 @@ -import { dedupeMixin } from '@open-wc/dedupe-mixin'; - -export const TIMEOUT_DURATION = 3000; - -export const AttributeRequiredMixin = dedupeMixin(superclass => class extends superclass { - - constructor() { - super(); - this._requiredAttributes = new Map(); - } - - firstUpdated(changedProperties) { - super.firstUpdated(changedProperties); - this._requiredAttributes.keys().forEach(name => this._validateRequiredAttribute(name)); - } - - updated(changedProperties) { - super.updated(changedProperties); - changedProperties.forEach((_oldValue, propName) => { - if (this._requiredAttributes.has(propName)) { - this._validateRequiredAttributeName(propName); - } - }); - } - - addRequiredAttribute(name, validator, message) { - this._requiredAttributes.set(name, { validator, message, timeout: null }); - } - - flushRequiredAttributeError(name) { - - if (!this._requiredAttributes.has(name) || !this.isConnected) return; - - const info = this._requiredAttributes.get(name); - info.timeout = null; - - const success = info.validator(); - if (!success) { - throw new Error(info.message); - } - - } - - flushRequiredAttributeErrors() { - this._requiredAttributes.keys().forEach(name => this.flushRequiredAttributeError(name)); - } - - _validateRequiredAttribute(name) { - const info = this._requiredAttributes.get(name); - clearTimeout(info.timeout); - info.timeout = setTimeout(() => this.flushRequiredAttributeError(name), TIMEOUT_DURATION); - } - -}); diff --git a/mixins/attribute-required/test/attribute-required-mixin.test.js b/mixins/attribute-required/test/attribute-required-mixin.test.js deleted file mode 100644 index 0d1bf4f8fa6..00000000000 --- a/mixins/attribute-required/test/attribute-required-mixin.test.js +++ /dev/null @@ -1,34 +0,0 @@ -import { AttributeRequiredMixin } from '../attribute-required-mixin.js'; -import { defineCE, expect, fixture, html, oneEvent } from '@brightspace-ui/testing'; -import { LitElement, render } from 'lit'; -import { restore, stub } from 'sinon'; - -const tag = defineCE( - class extends AttributeRequiredMixin(LitElement) { - static get properties() { - return { - foo: { type: String } - }; - } - constructor() { - super(); - this.addRequiredAttribute( - 'foo', - () => typeof(this.foo) === 'string' && this.foo.length > 0, - `The 'foo' attribute is required.` - ); - } - render() { return html`
hello
`; } - } -); - -describe('AttributeRequiredMixin', () => { - - it('should test things', async() => { - const elem = await fixture(`<${tag}>`); - expect(() => { - elem.flushRequiredAttributeErrors(); - }).to.throw(TypeError, 'The \'foo\' attribute is required.'); - }); - -}); diff --git a/mixins/labelled/labelled-mixin.js b/mixins/labelled/labelled-mixin.js index 0e88ba22278..b41b48e1f52 100644 --- a/mixins/labelled/labelled-mixin.js +++ b/mixins/labelled/labelled-mixin.js @@ -1,5 +1,6 @@ import { cssEscape } from '../../helpers/dom.js'; +import { PropertyRequiredMixin } from '../property-required/property-required-mixin.js'; const getCommonAncestor = (elem1, elem2) => { @@ -74,7 +75,7 @@ export const LabelMixin = superclass => class extends superclass { }; -export const LabelledMixin = superclass => class extends superclass { +export const LabelledMixin = superclass => class extends PropertyRequiredMixin(superclass) { static get properties() { return { @@ -96,20 +97,23 @@ export const LabelledMixin = superclass => class extends superclass { this.labelRequired = true; this._labelElem = null; this._missingLabelErrorHasBeenThrown = false; - this._validatingLabelTimeout = null; - } - - firstUpdated(changedProperties) { - super.firstUpdated(changedProperties); - this._validateLabel(); // need to check this even if "label" isn't updated in case it's never set + this.addRequiredProperty('label', { + message: defaultMessage => { + if (!this.labelledBy) return defaultMessage; + return `LabelledMixin: "${this.tagName.toLowerCase()}" is labelled-by="${this.labelledBy}", but its label is empty`; + }, + validator: hasValue => { + if (!this.labelRequired || hasValue) return true; + if (!this.labelledBy) return false; + return this._labelElem !== null; + } + }); } async updated(changedProperties) { super.updated(changedProperties); - if (changedProperties.has('label')) this._validateLabel(); - if (!changedProperties.has('labelledBy')) return; if (!this.labelledBy) { @@ -201,26 +205,4 @@ export const LabelledMixin = superclass => class extends superclass { } - _validateLabel() { - clearTimeout(this._validatingLabelTimeout); - // don't error immediately in case it doesn't get set immediately - this._validatingLabelTimeout = setTimeout(() => { - this._validatingLabelTimeout = null; - const hasLabel = (typeof this.label === 'string') && this.label.length > 0; - if (this.isConnected && !hasLabel) { - if (this.labelledBy) { - if (this._labelElem) { - this._throwError( - new Error(`LabelledMixin: "${this.tagName.toLowerCase()}" is labelled-by="${this.labelledBy}", but its label is empty`) - ); - } - } else { - this._throwError( - new Error(`LabelledMixin: "${this.tagName.toLowerCase()}" is missing a required "label" attribute`) - ); - } - } - }, 3000); - } - }; diff --git a/mixins/property-required/property-required-mixin.js b/mixins/property-required/property-required-mixin.js new file mode 100644 index 00000000000..3f13f2c222e --- /dev/null +++ b/mixins/property-required/property-required-mixin.js @@ -0,0 +1,108 @@ +import { dedupeMixin } from '@open-wc/dedupe-mixin'; + +const TIMEOUT_DURATION = 3000; + +export function createDefaultMessage(tagName, propertyName) { + return `${tagName}: "${propertyName}" attribute is required.`; +} + +export function createUndefinedPropertyMessage(tagName, propertyName) { + return `PropertyRequiredMixin: "${tagName.toLowerCase()}" has no property "${propertyName}".`; +} + +export function createInvalidPropertyTypeMessage(tagName, propertyName) { + return `PropertyRequiredMixin: only String properties can be required ("${tagName}" required property "${propertyName}".`; +} + +export const PropertyRequiredMixin = dedupeMixin(superclass => class extends superclass { + + constructor() { + super(); + this._allProperties = new Map(); + this._requiredProperties = new Map(); + this._initProperties(Object.getPrototypeOf(this)); + } + + firstUpdated(changedProperties) { + super.firstUpdated(changedProperties); + for (const name of this._requiredProperties.keys()) { + this._validateRequiredProperty(name); + } + } + + updated(changedProperties) { + super.updated(changedProperties); + this._requiredProperties.forEach((value, name) => { + const doValidate = changedProperties.has(name) || + value.dependentProps.includes(name); + if (doValidate) this._validateRequiredProperty(name); + }); + } + + addRequiredProperty(name, opts) { + + const prop = this._allProperties.get(name); + if (prop === undefined) { + throw new Error(createUndefinedPropertyMessage(this.tagName.toLowerCase(), name)); + } + + if (prop.type !== String) { + throw new Error(createInvalidPropertyTypeMessage(this.tagName.toLowerCase(), name)); + } + + opts = { + ...{ dependentProps: [], message: defaultMessage => defaultMessage, validator: hasValue => hasValue }, + ...opts + }; + + this._requiredProperties.set(name, { + attrName: prop.attribute || name, + dependentProps: opts.dependentProps, + message: opts.message, + thrown: false, + timeout: null, + validator: opts.validator + }); + + } + + flushRequiredPropertyErrors() { + for (const name of this._requiredProperties.keys()) { + this._flushRequiredPropertyError(name); + } + } + + _flushRequiredPropertyError(name) { + + if (!this._requiredProperties.has(name) || !this.isConnected) return; + + const info = this._requiredProperties.get(name); + clearTimeout(info.timeout); + info.timeout = null; + + const hasValue = this[name]?.constructor === String && this[name]?.length > 0; + const success = info.validator(hasValue); + if (!success) { + if (info.thrown) return; + info.thrown = true; + const defaultMessage = createDefaultMessage(this.tagName.toLowerCase(), info.attrName); + throw new TypeError(info.message(defaultMessage)); + } + + } + + _initProperties(base) { + if (base === null) return; + this._initProperties(Object.getPrototypeOf(base)); + for (const name in base.constructor.properties) { + this._allProperties.set(name, base.constructor.properties[name]); + } + } + + _validateRequiredProperty(name) { + const info = this._requiredProperties.get(name); + clearTimeout(info.timeout); + info.timeout = setTimeout(() => this._flushRequiredPropertyError(name), TIMEOUT_DURATION); + } + +}); diff --git a/mixins/property-required/test/property-required-mixin.test.js b/mixins/property-required/test/property-required-mixin.test.js new file mode 100644 index 00000000000..aab3251b42c --- /dev/null +++ b/mixins/property-required/test/property-required-mixin.test.js @@ -0,0 +1,219 @@ +import { createDefaultMessage, createInvalidPropertyTypeMessage, createUndefinedPropertyMessage, PropertyRequiredMixin } from '../property-required-mixin.js'; +import { defineCE, expect, fixture } from '@brightspace-ui/testing'; +import { LitElement } from 'lit'; + +const tagValid = defineCE( + class extends PropertyRequiredMixin(LitElement) {} +); + +const tagString = defineCE( + class extends PropertyRequiredMixin(LitElement) { + static properties = { + attr: { type: String } + }; + constructor() { + super(); + this.addRequiredProperty('attr'); + } + } +); + +describe('PropertyRequiredMixin', () => { + + it('should not throw if value is initially provided', async() => { + const elem = await fixture(`<${tagString} attr="value">`); + expect(() => elem.flushRequiredPropertyErrors()).to.not.throw(); + }); + + it('should not throw if value is provided before timeout', async() => { + const elem = await fixture(`<${tagString}>`); + elem.setAttribute('attr', 'value'); + await elem.updateComplete; + expect(() => elem.flushRequiredPropertyErrors()).to.not.throw(); + }); + + it('should not throw if element is removed from the DOM', async() => { + const elem = await fixture(`<${tagString}>`); + elem.parentNode.removeChild(elem); + expect(() => elem.flushRequiredPropertyErrors()).to.not.throw(); + }); + + it('should throw if value is initially missing', async() => { + const elem = await fixture(`<${tagString}>`); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tagString, 'attr')); + }); + + it('should throw if value is initially empty', async() => { + const elem = await fixture(`<${tagString} attr="">`); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tagString, 'attr')); + }); + + it('should throw if value is later removed', async() => { + const elem = await fixture(`<${tagString} attr="value">`); + elem.removeAttribute('attr'); + await elem.updateComplete; + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tagString, 'attr')); + }); + + it('should throw if value is later set to empty', async() => { + const elem = await fixture(`<${tagString} attr="value">`); + elem.setAttribute('attr', ''); + await elem.updateComplete; + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tagString, 'attr')); + }); + + it('should only throw once', async() => { + const elem = await fixture(`<${tagString} attr="">`); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tagString, 'attr')); + elem.setAttribute('attr', 'value'); + await elem.updateComplete; + elem.removeAttribute('attr'); + await elem.updateComplete; + expect(() => elem.flushRequiredPropertyErrors()).to.not.throw(); + }); + + it('should use camel-case name for multi-word attribute', async() => { + const tag = defineCE( + class extends PropertyRequiredMixin(LitElement) { + static properties = { + requiredAttr: { attribute: 'required-attr', type: String } + }; + constructor() { + super(); + this.addRequiredProperty('requiredAttr'); + } + } + ); + const elem = await fixture(`<${tag}>`); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tag, 'required-attr')); + }); + + it('should throw if no property is defined', async() => { + const elem = await fixture(`<${tagValid}>`); + expect(() => elem.addRequiredProperty('missing')) + .to.throw(createUndefinedPropertyMessage(tagValid, 'missing')); + }); + + it('should throw if type is non-String', async() => { + const tag = defineCE( + class extends PropertyRequiredMixin(LitElement) { + static properties = { + attr: { type: Boolean } + }; + } + ); + const elem = await fixture(`<${tag}>`); + expect(() => elem.addRequiredProperty('attr')) + .to.throw(createInvalidPropertyTypeMessage(tag, 'attr')); + }); + + it('should use custom message', async() => { + const tag = defineCE( + class extends PropertyRequiredMixin(LitElement) { + static properties = { + attr: { type: String } + }; + constructor() { + super(); + this.addRequiredProperty('attr', { + message: () => 'custom message' + }); + } + } + ); + const elem = await fixture(`<${tag}>`); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, 'custom message'); + }); + + it('should use custom validator', async() => { + const tag = defineCE( + class extends PropertyRequiredMixin(LitElement) { + static properties = { + attr: { type: String } + }; + constructor() { + super(); + this.addRequiredProperty('attr', { + validator: () => this.attr === 'valid' + }); + } + } + ); + const elem = await fixture(`<${tag} attr="oh no">`); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tag, 'attr')); + }); + + it('should pass hasValue to custom validator', async() => { + const tag = defineCE( + class extends PropertyRequiredMixin(LitElement) { + static properties = { + attr: { type: String } + }; + constructor() { + super(); + this.addRequiredProperty('attr', { + validator: hasValue => hasValue + }); + } + } + ); + const elem = await fixture(`<${tag}>`); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tag, 'attr')); + }); + + it('should validate when dependent property changes', async() => { + const tag = defineCE( + class extends PropertyRequiredMixin(LitElement) { + static properties = { + attr1: { type: String }, + attr2: { type: String } + }; + constructor() { + super(); + this.addRequiredProperty('attr1', { + dependentProps: ['attr2'], + validator: () => this.attr2 === 'valid' + }); + } + } + ); + const elem = await fixture(`<${tag} attr1="value" attr2="valid">`); + expect(() => elem.flushRequiredPropertyErrors()).to.not.throw(); + elem.removeAttribute('attr2'); + await elem.updateComplete; + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tag, 'attr1')); + }); + + it('should work in a subclass/mixin', async() => { + const TestMixin = superclass => class extends PropertyRequiredMixin(superclass) { + static properties = { + attr1: { type: String } + }; + constructor() { + super(); + this.addRequiredProperty('attr1'); + } + }; + const tagMixin = defineCE( + class extends TestMixin(LitElement) { + static properties = { + attr2: { type: String } + }; + } + ); + const elem = await fixture(`<${tagMixin}>`); + expect(() => elem.flushRequiredPropertyErrors()) + .to.throw(TypeError, createDefaultMessage(tagMixin, 'attr1')); + }); + +});