Skip to content

Commit

Permalink
use both element property and attribute to set the value on the element
Browse files Browse the repository at this point in the history
  • Loading branch information
ECorreia45 committed Dec 8, 2024
1 parent 484631b commit ae52a91
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 41 deletions.
43 changes: 21 additions & 22 deletions docs/documentation/capabilities/form-controls.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class TextField extends WebComponent {
'value',
'placeholder',
'pattern',
'disabled',
'required',
'error',
]
Expand All @@ -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`
<input
${inputAttrs}
part="text-input"
type="text"
ref="input"
onchange="${(event) => this.handleChange(event.target.value)}"
placeholder="${this.props.placeholder}"
disabled="${this.props.disabled}"
pattern="${this.props.pattern}"
required="${this.props.required}"
value="${this.props.value}"
/>
`
}
Expand Down Expand Up @@ -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);
}
}
```
Expand All @@ -173,6 +174,7 @@ class TextField extends WebComponent {

handleChange = (value, report = true) => {
this.internals.setFormValue(value);
this.value = value;

const [inputField] = this.refs['input'];

Expand Down Expand Up @@ -206,7 +208,7 @@ class TextField extends WebComponent {
...

formAssociatedCallback(form) {
this.internals.setFormValue(this.props.value());
this.handleChange(this.props.value(), false);
}

...
Expand All @@ -227,7 +229,7 @@ class TextField extends WebComponent {
...

formDisabledCallback(disabled) {
this.refs['input'][0].disabled = disabled;
this.disabled = disabled;
}

...
Expand All @@ -245,8 +247,7 @@ class TextField extends WebComponent {
...

formResetCallback(form) {
this.handleChange('')
this.refs['input'][0].value = ''
this.handleChange('', false)
}

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

Expand All @@ -290,6 +290,7 @@ class TextField extends WebComponent {
static observedAttributes = [
'value',
'placeholder',
'disabled',
'pattern',
'error',
'required',
Expand All @@ -316,6 +317,7 @@ class TextField extends WebComponent {
placeholder = ''
value = ''
pattern = ''
disabled = false
required = false
error = 'Invalid field value'

Expand All @@ -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']

Expand All @@ -357,17 +358,15 @@ class TextField extends WebComponent {
}

render() {
const { error, ...inputAttrs } = this.props

return html`
<input
${inputAttrs}
part="text-input"
type="text"
ref="input"
onchange="${(event) => this.handleChange(event.target.value)}"
placeholder="${this.props.placeholder}"
disabled="${this.props.disabled}"
pattern="${this.props.pattern}"
required="${this.props.required}"
value="${this.props.value}"
/>
`
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
31 changes: 30 additions & 1 deletion src/html.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
<input type="text" value="${value}" ref="input" />
`.render(document.body);

expect(document.body.innerHTML).toBe('<input type="text" value="">');

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', () => {
Expand All @@ -854,7 +883,7 @@ describe('html', () => {
el.render(document.body)

expect(document.body.innerHTML).toBe(
'<div data-test-map="[object Map]" data-test-val="[object Object]"></div>'
'<div data-test-map="{}" data-test-val="{&quot;val&quot;:12}"></div>'
)
})

Expand Down
41 changes: 24 additions & 17 deletions src/utils/set-element-attribute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit ae52a91

Please sign in to comment.