From ae52a911e4bdfd6bcfeabae2b094bc3fcb0c1252 Mon Sep 17 00:00:00 2001 From: Elson Correia Date: Sun, 8 Dec 2024 00:31:26 -0500 Subject: [PATCH] use both element property and attribute to set the value on the element --- .../capabilities/form-controls.md | 43 +++++++++---------- package.json | 2 +- src/html.spec.ts | 31 ++++++++++++- src/utils/set-element-attribute.ts | 41 ++++++++++-------- 4 files changed, 76 insertions(+), 41 deletions(-) diff --git a/docs/documentation/capabilities/form-controls.md b/docs/documentation/capabilities/form-controls.md index 7dfdc840..1721d863 100644 --- a/docs/documentation/capabilities/form-controls.md +++ b/docs/documentation/capabilities/form-controls.md @@ -21,6 +21,7 @@ class TextField extends WebComponent { 'value', 'placeholder', 'pattern', + 'disabled', 'required', 'error', ] @@ -30,26 +31,26 @@ class TextField extends WebComponent { value = '' pattern = '' required = false + disabled = false error = 'Invalid field value.' handleChange = (value) => { + this.value = value // dispatch a change event with the input field value this.dispatch('change', { value }) } // render the component content render() { + const { error, ...inputAttrs } = this.props + return html` ` } @@ -148,7 +149,7 @@ class TextField extends WebComponent { formAssociatedCallback(form) { // register value received from props - this.internals.setFormValue(this.props.value()); + this.internals.setFormValue(this.props.value(), false); } } ``` @@ -173,6 +174,7 @@ class TextField extends WebComponent { handleChange = (value, report = true) => { this.internals.setFormValue(value); + this.value = value; const [inputField] = this.refs['input']; @@ -206,7 +208,7 @@ class TextField extends WebComponent { ... formAssociatedCallback(form) { - this.internals.setFormValue(this.props.value()); + this.handleChange(this.props.value(), false); } ... @@ -227,7 +229,7 @@ class TextField extends WebComponent { ... formDisabledCallback(disabled) { - this.refs['input'][0].disabled = disabled; + this.disabled = disabled; } ... @@ -245,8 +247,7 @@ class TextField extends WebComponent { ... formResetCallback(form) { - this.handleChange('') - this.refs['input'][0].value = '' + this.handleChange('', false) } ... @@ -270,8 +271,7 @@ class TextField extends WebComponent { if (mode == 'restore') { // expects a state parameter in the form 'controlMode/value' const [controlMode, value] = state.split('/'); - this.handleChange(value) - this.refs['input'][0].value = value + this.handleChange(value, false) } } @@ -290,6 +290,7 @@ class TextField extends WebComponent { static observedAttributes = [ 'value', 'placeholder', + 'disabled', 'pattern', 'error', 'required', @@ -316,6 +317,7 @@ class TextField extends WebComponent { placeholder = '' value = '' pattern = '' + disabled = false required = false error = 'Invalid field value' @@ -324,24 +326,23 @@ class TextField extends WebComponent { } formDisabledCallback(disabled) { - this.refs['input'][0].disabled = disabled + this.disabled = disabled } formResetCallback() { - this.handleChange('') - this.refs['input'][0].value = '' + this.handleChange('', false) } formStateRestoreCallback(state, mode) { if (mode == 'restore') { const [controlMode, value] = state.split('/') - this.handleChange(value) - this.refs['input'][0].value = value + this.handleChange(value, false) } } handleChange = (value, report = true) => { this.internals.setFormValue(value) + this.value = value const [inputField] = this.refs['input'] @@ -357,17 +358,15 @@ class TextField extends WebComponent { } render() { + const { error, ...inputAttrs } = this.props + return html` ` } diff --git a/package.json b/package.json index 12fdcf8e..d91ed4b7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@beforesemicolon/markup", - "version": "1.2.0", + "version": "1.3.0", "description": "Reactive HTML Templating System", "engines": { "node": ">=18.16.0" diff --git a/src/html.spec.ts b/src/html.spec.ts index d89fafc9..bd208748 100644 --- a/src/html.spec.ts +++ b/src/html.spec.ts @@ -828,6 +828,35 @@ describe('html', () => { expect(countUpSpy).toHaveBeenCalledTimes(1); expect(countUpSpy).toHaveBeenCalledWith(expect.any(Event)); }) + + it('should handle input value', () => { + const [value, setValue] = state(""); + + const temp = html` + + `.render(document.body); + + expect(document.body.innerHTML).toBe(''); + + const [inputField] = temp.refs['input'] as HTMLInputElement[]; + + expect(inputField.getAttribute('value')).toBe('') + expect(inputField.value).toBe('') + + setValue('works') + + jest.advanceTimersToNextTimer(5) + + expect(inputField.getAttribute('value')).toBe('works') + expect(inputField.value).toBe('works') + + setValue('') + + jest.advanceTimersToNextTimer(5) + + expect(inputField.getAttribute('value')).toBe('') + expect(inputField.value).toBe('') + }) }) it('should handle primitive attribute value', () => { @@ -854,7 +883,7 @@ describe('html', () => { el.render(document.body) expect(document.body.innerHTML).toBe( - '
' + '
' ) }) diff --git a/src/utils/set-element-attribute.ts b/src/utils/set-element-attribute.ts index ab5c9b78..5c5e9437 100644 --- a/src/utils/set-element-attribute.ts +++ b/src/utils/set-element-attribute.ts @@ -13,26 +13,33 @@ export const setElementAttribute = ( value: unknown ) => { if (value !== undefined && value !== null) { - // take care of web component elements with prop setters - if (el.nodeName.includes('-') && !isPrimitive(value)) { - const descriptor = - // describe the property directly on the object - Object.getOwnPropertyDescriptor(el, key) ?? - // describe properties defined as setter/getter by checking the prototype - Object.getOwnPropertyDescriptors(Object.getPrototypeOf(el))[key] + const descriptor = + Object.getOwnPropertyDescriptor(el, key) ?? + // describe properties defined as setter/getter by checking the prototype + Object.getOwnPropertyDescriptors(Object.getPrototypeOf(el))[key] + const isWritable = + descriptor?.writable || typeof descriptor?.set === 'function' + // Using setAttribute() to modify certain attributes, most notably value in XUL, works inconsistently, + // as the attribute specifies the default value. To access or modify the current values, you should use + // the properties. For example, use elt.value instead of elt.setAttribute('value', val). + // https://stackoverflow.com/questions/29929797/setattribute-doesnt-work-the-way-i-expect-it-to + if (isWritable) { + // @ts-expect-error Cannot assign to X because it is a read-only property. + el[key] = value + } - if (descriptor?.writable || typeof descriptor?.set === 'function') { - // @ts-expect-error Cannot assign to X because it is a read-only property. - if (el[key] !== value) el[key] = value - } else { - const v = jsonStringify(value) - if (v !== el.getAttribute(key)) el.setAttribute(key, v) - } + const strValue = jsonStringify(value) - return + if ( + // in case !isWritable or setting the property did not also update the attribute + strValue !== el.getAttribute(key) && + // only set primitive values + // only set non-primitive values on non-components + // only set non-primitive values on web components if the property is !isWritable + (isPrimitive(value) || !el.nodeName.includes('-') || !isWritable) + ) { + el.setAttribute(key, strValue) } - - if (value !== el.getAttribute(key)) el.setAttribute(key, String(value)) } else { el.removeAttribute(key) }