New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mithril 0.2.0 textarea not updating when expected if not storing its value to a model #701

Closed
pdfernhout opened this Issue Jun 30, 2015 · 5 comments

Comments

Projects
None yet
3 participants
@pdfernhout
Contributor

pdfernhout commented Jun 30, 2015

This is essentially to reopen Issue #95 by ldimi. Here is a JSFiddle that demonstrates an issue with textarea updating: http://jsfiddle.net/pdfernhout/wv46fkg0/

The test seems to show that a textarea is not being updated to overwrite its user-entered contents unless the related model value itself changes (which then causes Mithril to decide the textarea's value is out-of-date). This means the textarea could potentially get out of step with what is in the model. It seems possible that textareas only work OK in most in-practice cases because their models are constantly changing to different values as they are edited and store those changes into the model? Granted, the behavior demonstrated in the fiddle may not be a common situation because the behavior will only manifest itself if an application does not respond to every change in a textarea's contents in a way that updates the value of the textarea. However, as the fiddle shows, a regular text input works as one might expect, so there is an inconsistency here.

In the application I'm working on, this issue manifested when initially setting a item-detail textarea to an empty string via value and then manually entered text into the textarea via the browser. The application code (intentionally) discarded the change because an associated display item was not yet selected. So, the textarea's internal value was not being updated by the application in the view method as the textarea's text was changed by the user, as would otherwise normally be the case. (Yes, the textarea should ideally be disabled if the data is not available -- I just noticed this issue while working towards that ideal.) The issue became apparent when a subsequent change in the data model by setting value on the textarea again to an empty string for an available data item was not reflected in the application GUI. I think this is because Mithril 0.2.0 apparently decided that the textarea must still be blank because its value was originally set that way (despite its actual contents being changed by the user). When I replaced the textarea with a text input, the application worked as I initially expected, with the displayed value in the input cleared when selecting an available item.

The solution is probably similar to what ldimi outlined. In setAttributes, the test:

else if (attrName === "value" && tag === "input" && node.value != dataAttr) {

should probably be:

  else if (attrName === "value" && (tag === "input" || tag === "textarea") && node.value != dataAttr) {

I say "probably" because since a textarea can take internal text elements, as well as be set by "value", I'm not sure about any other implications in that other case. I tested the change locally and with the change the textarea updated as I expected (back to empty when tabbing out caused a redraw). But I was only setting the value, not the internal text components. Also, I'm not sure what the performance implications are. I'd agree this is an edge case, since most applications would presumably have a valid model to store data in that closely tracks the textarea's current contents, and so Mithril 0.2.0's current updating strategy might be good enough in that case. However, it is also an edge case that took hours of troubleshooting and research to isolate down to this test case and this understanding -- so ideally others could be spared that, since this edge case may be a common situation while developing an app incrementally, even if a finished app may avoid it. Still, on the plus side, I know a bit more about Mithril internals now from debugging this. :-) And a big advantage of Mithril is that it is much easier to debug than many other GUI frameworks.

Also, I wonder if this issue is to any extent a regression error introduce by removing the check in setAttributes of: "|| node === window.document.activeElement" which was present in Mithril 0.1.14? Presumably that might have forced the update of a recently-used textarea's value when it was the current widget? I'm not saying to go back to that approach, just that textarea updating might have worked differently back then.

@pdfernhout

This comment has been minimized.

Show comment
Hide comment
@pdfernhout

pdfernhout Jun 30, 2015

Contributor

It looks like issue #691 related to checkboxes may have the same cause as this one. It is also possible that radiobuttons, and selects may have this issue too (since they also have either checked or a value that would need to be tracked). I outlined a somewhat more complex test and set code in a response to that issue which includes those other types.

Contributor

pdfernhout commented Jun 30, 2015

It looks like issue #691 related to checkboxes may have the same cause as this one. It is also possible that radiobuttons, and selects may have this issue too (since they also have either checked or a value that would need to be tracked). I outlined a somewhat more complex test and set code in a response to that issue which includes those other types.

@isiahmeadows

This comment has been minimized.

Show comment
Hide comment
@isiahmeadows

isiahmeadows Jul 7, 2015

Collaborator

Is this still the case with Mithril 0.4?

Collaborator

isiahmeadows commented Jul 7, 2015

Is this still the case with Mithril 0.4?

@pdfernhout

This comment has been minimized.

Show comment
Hide comment
@pdfernhout

pdfernhout Jul 11, 2015

Contributor

@impinball The current released version of Mithril at GitHub seems to be 0.2.0. That is the one which demonstrates the issue and which I am using myself. I am not aware of a Mithril 0.4 being available yet, or even in development?

BTW, we are discussing the root of this issue further in issue #691 as it applies to almost any user-modifiable property including checked, scrollTop, selectionStart, etc.. A big decision is whether this behavior is a "feature" or a "bug" and how to move forward on changing it and/or documenting it.

Contributor

pdfernhout commented Jul 11, 2015

@impinball The current released version of Mithril at GitHub seems to be 0.2.0. That is the one which demonstrates the issue and which I am using myself. I am not aware of a Mithril 0.4 being available yet, or even in development?

BTW, we are discussing the root of this issue further in issue #691 as it applies to almost any user-modifiable property including checked, scrollTop, selectionStart, etc.. A big decision is whether this behavior is a "feature" or a "bug" and how to move forward on changing it and/or documenting it.

@pelonpelon

This comment has been minimized.

Show comment
Hide comment
@pelonpelon

pelonpelon Jul 11, 2015

Contributor

@pdfernhout Maybe you should close this issue with a forwarding link to the other discussion.

Contributor

pelonpelon commented Jul 11, 2015

@pdfernhout Maybe you should close this issue with a forwarding link to the other discussion.

@pdfernhout

This comment has been minimized.

Show comment
Hide comment
@pdfernhout

pdfernhout Jul 12, 2015

Contributor

I'm closing this issue as @pelonpelon suggested since it has the same underlying cause as issue #691.

Contributor

pdfernhout commented Jul 12, 2015

I'm closing this issue as @pelonpelon suggested since it has the same underlying cause as issue #691.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment