Fixed update input value by attr binding

[Den4ik]

Reference comment https://github.com/magento/magento2/pull/33027#issuecomment-849443012

This PR fixes input element binding for value if it's done by

attr: {
    value: someFunc
}

Binding to value in attr section will not update input field because it’s bug or feature of HTMLInputElement implementation. Visible value linked to HTMLInputElement value property. attr section use setAttribute method of Element api implementation and this is don’t effect to value property. You may try experiment on this page, for example: https://developer.mozilla.org/en-US/docs/Web/API/Element

document.querySelectorAll("[id='main-q']")[0].value
""
document.querySelectorAll("[id='main-q']")[0].getAttribute('value')
null
document.querySelectorAll("[id='main-q']")[0].setAttribute('value', 1)
undefined
document.querySelectorAll("[id='main-q']")[0].value=2
2
document.querySelectorAll("[id='main-q']")[0].getAttribute('value')
"1"
document.querySelectorAll("[id='main-q']")[0].value
"2"

[Den4ik]

@mbest Could you pay attention to this PR?

[webketje]

Have you tried

attr: {
    value: someFunc()
}

Or

attr: {
    value: someFunc
}

where someFunc is a (pure)computed. I think it works as expected, only observables are unwrapped.

[Den4ik]

@webketje what about observable variable? It's not working as expected

[Den4ik]

Also as I mentioned attribute value and main import value has different scope

[fastfasterfastest]

Is there an issue discussing this?

I am not sure I understand, but it appears that you are expecting the attr binding (when used with value) to use the value property of the DOM element rather than the value attribute of the DOM element.

The value attribute of a DOM element is not the same thing as the value property.

If you want to bind to the value property, use the value binding. If you want to bind to the value attribute, use the attr binding.

To me, things appear to be working correctly.

[webketje]

Actually yeah.. second @fastfasterfastest 's opinion.. I don't see any valid use case for using attr: { value } as opposed to simply the value binding. attr.value should only set the initial value, as in the HTML spec, and so your tests are working as expected in the first comment

[Den4ik]

@webketje @fastfasterfastest

attr.value should only set the initial value

Could not agree with it because:

• attr: { value: observableVariable } allow define observable variable and as result I expect to see changes in input field after the change of observable variable.
• knockout don't have any restrictions for ⬆️
• documentation don't have any information about this behaviour and that input field will not be updated with actual attribute value

It's related to https://github.com/magento/magento2/issues/32456, https://github.com/magento/magento2/pull/33027, https://github.com/magento/magento2/pull/33591

Magento 2 has hundreds the usage of value in attr and each one it potential problem.

[karimayachi]

@fastfasterfastest is right.

The value property of input elements is not reflected back to the attribute. This is no bug, but per specification. Also the attribute is only reflected to the property if the property is undefined. As soon as the property has a value (by Knockout binding, or by typing in the input field, or anything that triggers an onchange event) changes to the attribute are ignored. If this is per spec I don't know, but it is how browsers handle it.

This all has nothing to do with Knockout.

attr: { value: observableVariable } allow define observable variable and as result I expect to see changes in input field after the change of observable variable.

This works as expected. You bind an observable to the attribute and the attribute is updated along with changes to observable. The fact that changes to the attribute are not reflected to the value-property is beyond Knockouts control and you should use the value-binding for that.

I don't know about the Magento issues, but if I understand correctly they use the attr-binding in stead of the value-binding? I would say that that is an issue over on their side, not on the Knockout side of things.

Furthermore, I looked at your PR. It introduces many assumptions in otherwise generic code. That is bound to break things. I would strongly advise against these changes.

Regards

[mbest]

I agree with the other comments. For an input element use the value binding instead of attr.