Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#18563 closed enhancement (fixed)

NumberTextBox doesn't fill trailing zeros when using constraints: {places: 2}

Reported by: Michael Schall Owned by: Bill Keese <bill@…>
Priority: undecided Milestone: 1.11
Component: Dijit - Form Version: 1.10.4
Keywords: Cc:
Blocked By: Blocking:

Description

The NumberTextBox? (and children) require the user to enter trailing zeros to fulfill the number of "places" is specified. For the CurrencyTextBox?, the places is add for you by the setting currency attribute (i.e. currency: 'USD'). An example showing constraints with places set available on ​jsFiddle.

This means that a widget setup like the following

<input type="text" data-dojo-type="dijit/form/NumberTextBox"
    data-dojo-props="constraints:{places:2}" value="0" />

Has has the following issues:

  • 1 is not formatted to 1.00 on blur and shows a validation
  • 1.1 is not formatted to 1.10 on blur and shows a validation

Both of these work if you just use {pattern:'##0.00'}. As mentioned, CurrencyTextBox? forces a places, so that is not an option. The places setting also gives immediate validation when you go over the desired length which is very nice for the user.

I think I have resolved the issues with the following changes:

  • NumberTextBox.js

     
    179179                        if(this.editOptions && this.focused && isNaN(v)){
    180180                                v = this._parser(value, constraints); // parse w/o editOptions: not technically needed but is nice for the user
    181181                        }
     182                        if (!this.focused && isNaN(v)  && constraints.places != null  /* or undefined */) {
     183                                var maxPlaces = Number(constraints.places.toString().split(",").pop()); // handle number and range
     184                                v = this._parser(value, lang.mixin({}, constraints, { places: "0," + maxPlaces }));
     185                        }
    182186                        return v;
    183187                },
    184188
     
    211214                },
    212215
    213216                _setBlurValue: function(){
    214                         var val = lang.hitch(lang.delegate(this, { focused: true }), "get")('value'); // parse with editOptions
     217                        var val = this.get('value');
    215218                        this._setValueAttr(val, true);
    216219                },

The risk here is changing the _setBlurValue to get the value as "focused: false". I'm sure it was done this way for a reason, but I'm not sure why we would want to parse as focused on blur anyway. I'm sure I'm missing something. Another option is to pass an inBlur flag along to resolve the issue like the following change:

  • NumberTextBox.js

     
    179179                        if(this.editOptions && this.focused && isNaN(v)){
    180180                                v = this._parser(value, constraints); // parse w/o editOptions: not technically needed but is nice for the user
    181181                        }
     182                        if (this.inBlur && isNaN(v) && constraints.places != null  /* or undefined */) {
     183                                var maxPlaces = Number(constraints.places.toString().split(",").pop()); // handle number and range
     184                                v = this._parser(value, lang.mixin({}, constraints, { places: "0," + maxPlaces }));
     185                        }
    182186                        return v;
    183187                },
    184188
     
    211214                },
    212215
    213216                _setBlurValue: function(){
    214                         var val = lang.hitch(lang.delegate(this, { focused: true }), "get")('value'); // parse with editOptions
     217                        var val = lang.hitch(lang.delegate(this, { focused: true, inBlur: true }), "get")('value'); // parse with editOptions
    215218                        this._setValueAttr(val, true);
    216219                },

This change makes entering numbers more user friendly across all NumberTextBoxes?.

Change History (18)

comment:1 Changed 5 years ago by bill

Component: GeneralDijit - Form

I don't remember... you might be missing something with localization? Or just that in the US locale 1,000 is displayed as 1000 when focused, and 1,000 when blurred.

I agree it would be nice to automatically add the trailing zeros on blur, rather than giving a validation error. Are you sure you can't already get this behavior by setting editOptions in addition to constraints?

comment:2 Changed 5 years ago by Michael Schall

I don't see anything helpful in editOptions. I also don't think we want to use anything from editOptions as we can't add zeros while we are editing?

comment:3 Changed 5 years ago by Michael Schall

I did some testing with a CurrencyTextBox? and my first change above (not adding the inBlur).

  • Typing 1000 correctly formatted to $1,000.00 on blur
  • Editing with $1,000.00 correctly formatted to 1000.00 onfocus and back to $1,000.00 onblur
<input type="text" maxlength="10" required="required" 
    data-dojo-type="dijit/form/CurrencyTextBox" 
    data-dojo-props="
        constraints: {min: 0, max: 9999999.99}, 
        currency: 'USD', trim: true,
        intermediateChanges: true" />

I'm not sure how/what to test for localization.

comment:4 Changed 5 years ago by bill

For localization, test_validate.html has a field with a non-english locale (specifically, "euro currency (fixed lang: de-de)"). That's a test for currency, but you could change it to a test for plain numbers. I was just talking about how the formatted value is 1.000,00 rather than 1,000.00.

comment:5 Changed 5 years ago by Michael Schall

I was able to test test_validate.html and my code seems to work fine on the "Annual Income" section. All USD, Euro, and Euro in the de-de lang format to what I believe to be correct onblur.

comment:6 Changed 5 years ago by bill

OK. But anyway, I still think this can be handled with the current code by editOptions. The idea is that you set constraints to require places:2, but then editOptions overrides that so places:2 is not required during editing. That's my memory anyway.

comment:7 Changed 5 years ago by Michael Schall

I will look into that, but if I understand your suggestion, I think that would get rid of the nice validation message while editing if you go past the 2 digits.

I have a pull request ready. It is my first, so not sure how this normally works. Do you want the pull request to your wkeese fork, or against the main one?

comment:8 Changed 5 years ago by bill

Oh, it should go against the main one. And it should include a regression test (plus the existing regression tests should still pass).

comment:9 Changed 5 years ago by bill

PS: Can you set editOptions to allow a maximum of 2 decimal places?

Version 0, edited 5 years ago by bill (next)

comment:10 Changed 5 years ago by Michael Schall

That would work... However it would be nice if dijit supported this behavior "out of the box" rather than having to set editOptions on all NumberTextBoxes? (and children) or using inheritance. Thoughts?

Can you point me to an example of a dijit regression test? Is it just another box or set of boxes on the text_validate.html? Is there automated tests for dijit? If so, is there any documentation on how to setup the automated tests (on Windows)?

comment:11 Changed 5 years ago by bill

I agree that would be nicer. On a related issue, for version 2 I had always wanted to separate formatting options from constraints.

As for the regression tests, they are easy to run. You just need to have java installed. And of course, a web server (like apache) itself, since as you probably know dojo doesn't work well over the file:// protocol. For some of the tests you need PHP enabled, but I don't think that's necessary for the form tests.

You run tests from the browser, either load a single test like http://localhost/dijit/tests/form/robot/ValidationTextBox.html, or to run the whole suite of form tests do http://localhost/dijit/tests/form/runTests.html. Your URL will vary depending on the paths on your system.

comment:12 Changed 5 years ago by Michael Schall

Bill - I tried getting the current test working (without my changes) on my local box and many of them are failing. I also tried just running them from the nightly tests with the same results. I went to the IRC and snover said it was very hard to get the test running and passing. snover suggested I try installing Java 6 on a VM and that "might" work. Do you have test running and passing? What is your setup for test.

comment:13 Changed 5 years ago by bill

Don't believe everything you hear. The tests are all running fine for me, with the latest version of java (V8, not V6), except for some occasional intermittent failures. I did forget to mention a few things (as listed in https://github.com/dojo/util/blob/master/doh/robot/README):

  1. You might need to add the server to the list of java security exception sites
  2. You'll probably get a security prompt the first time. Or maybe it's just an icon that shows up in the address bar, and you have to click it.
  3. Since the tests take control of the machine's mouse and keyboard it's better to run in a VM or a dedicated machine. Also, I don't know if they work on mac or linux; you should try windows.

I'm not sure what issue you are seeing, so it's hard to respond more explicitly.

comment:14 Changed 5 years ago by Michael Schall

I had to change my approach slightly, but my change does pass current tests (after changing 2 to expect my update) and I wrote 2 new tests. Let me know what you think... pull request

comment:15 Changed 5 years ago by bill

Milestone: tbd1.11

comment:16 Changed 5 years ago by bill

For the record, the way to get the desired behavior w/the existing code is:

<input type="text" data-dojo-type="dijit/form/NumberTextBox"
    data-dojo-props="constraints:{places:2}, editOptions:{places:'0,2'}">

comment:17 Changed 4 years ago by Bill Keese <bill@…>

Owner: set to Bill Keese <bill@…>
Resolution: fixed
Status: newclosed

In b9e77065ffa187a1373722d3d5917f0b44132540/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:18 Changed 4 years ago by Bill Keese <bill@…>

In 972a42b8afa31b16093fa32dc31cbe45b2aa1102/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.