Opened 6 years ago

Closed 4 years ago

#17182 closed defect (patchwelcome)

bidi binding on NumberTextBox

Reported by: Stefan Meyer Owned by: Ed Chatelain
Priority: undecided Milestone: 1.11
Component: DojoX MVC Version: 1.9.0
Keywords: Cc: Akira Sudoh
Blocked By: Blocking:

Description

I did not get any replies to this on the mailing list. I think this is a serious issue:

when binding a validation-capable dijit to a dojo.Stateful via dojox/mvc I ran into issues with invalid values. Specifically values that cannot be converted to the target type. E.g. The user enters non-numerical characters in a NumberTextBox?. In that case the value of the dijit is set to blank by the NumberTextBox?. The bidi binding will result in the dojo.Stateful being updated and that change will trigger an update in the dijit, which will actually also affect the displayed value and erase the user input.

At the end of the day the user enters "hello". When moving the focus away the value is erased.

I see two points to tackle the problem:

  1. Make the dijit not update its value when the user input is invalid.

I used this approach to fix my issues. Unfortunately validation is performed on the property value. So I had to add an extra check before actually setting the value. That fix clearly violates the DRY priniciple.

  1. Add condition to binding

If the binding had a condition attached so that changes only get propagated when the condition is met. The problem would be solved by simply checking the dijit's state. If the state is "Error" the binding is inactive.

What is your opinion?

Best, Stefan

Change History (10)

comment:1 Changed 6 years ago by Stefan Meyer

I had an interesting conversation with Akira Sudo on the mailing list. It seems that there is not a real consensus about where validation logic should be located.

To fix my issue I had to make the converter aware of the source (the widget) and throw an exception when the dijit's state is "Error". Passing the dijit to the converter is pretty cumbersome.

The fix I suggest is to pass the source/dijit to the converter's parse method. Please let me know if this is a viable solution for you.

Best, Stefan

comment:2 Changed 6 years ago by Ed Chatelain

Owner: set to Ed Chatelain
Status: newassigned

comment:3 Changed 6 years ago by Ed Chatelain

Cc: Akira Sudoh added

comment:4 Changed 6 years ago by Akira Sudoh

Here's the mailing list thread Stefan referred to: http://thread.gmane.org/gmane.comp.web.dojo.user/75992/

comment:5 Changed 6 years ago by Akira Sudoh

I think suggestion 1. needs feedback from Bill as it's about widgets' design. I think the best way to address 2. is application to attach a data converter to the data binding via .transform(), like:

{
    parse: function(value, constraints){
        if(this.target.state == "Error"){
            throw new Error("The input value is not valid");
        }
        return value;
    }
}

this in data converter function is: {source: sourceStateful, target: targetStateful}. sourceStateful typically is a data model, targetStateful typically is a widget.

comment:6 Changed 6 years ago by Stefan Meyer

If "this" is guaranteed to be that object then that solves my problem. But making that information available via a parameter or an initializ function would be clearly better because firstly it would actually be documented in a standard way and I would not have run into problems. Secondly I think relying on "this" is only acceptable if we are talking about a method in an object an "this" refers to that object. Otherwise it feels like a bad pattern or simply a hack.

comment:7 Changed 6 years ago by Akira Sudoh

We can definitely update the document in the next iteration, as the data converter (from the beginning) is called with lang.hitch({source: sourceStateful, target: targetStateful}, formatter): https://github.com/dojo/dojox/blob/e393c9e116dc7271c698f9a2bfc1310acfcc77c6/mvc/sync.js#L169-L171

comment:8 Changed 6 years ago by bill

FWIW, w.r.t. suggestion (1), I think the root problem is that NumberTextBox?'s value is a number instead of a string... obviously that works well most of the time, but not when the user types "hello". So I regret that design decision.

To address your actual suggestion to

Make the dijit not update its value when the user input is invalid.

... I'm not sure how that would help. If the user types "123", and then replaces it with "hello", the NumberTextBox?'s value is still reported as 123? It sounds problematic.

comment:9 Changed 6 years ago by Stefan Meyer

I personally find that invalid values should not be propagated. My experience with a lot of libraries shows that this is a widespread Ansatz. Surely you should have access to the invalid value though. So my solution 1. is maybe not so good after all.

But some dijits validate after they send change events. I opened a ticket because it makes the solution 2. impossible(https://bugs.dojotoolkit.org/ticket/17436).

comment:10 Changed 4 years ago by dylan

Milestone: tbd1.11
Resolution: patchwelcome
Status: assignedclosed

dojox/mvc is not actively maintained. If someone wants to work on pull requests for the issues, we'll consider them.

Note: See TracTickets for help on using tickets.