Opened 6 years ago

Closed 6 years ago

#17923 closed defect (fixed)

[patch][cla] NumberTextBox does not properly support leading zeros in editOptions.pattern

Reported by: Terence Kent Owned by: bill
Priority: high Milestone: 1.10
Component: Dijit - Form Version: 1.9.3
Keywords: Cc:
Blocked By: Blocking:

Description

When a NumberTextBox? contains an editOptions.pattern which expects leading 0s, and the constraints.min is greater than 0, the _isValidSubset() method doesn't work properly.

For example, with editOptions.pattern = "000", and constraints = { min: 1, max: 500}, the user will get see error indicators while typing the first two keys when entering "004".

I've put together a jsfiddle showing both the issue and a patch for the issue here: http://jsfiddle.net/bYG37/4/

I'm currently using the aspect included to get around the problem.

Change History (8)

comment:1 Changed 6 years ago by Terence Kent

I've updated the patch we're using to include significant digit checking and fix a few bugs that we ran into. Unfortunately, I can't edit my issue description, so sorry for the spam.

I'll keep the most updated example here: http://jsfiddle.net/terencekent/SktJ8/

comment:2 Changed 6 years ago by dylan

Owner: set to bill
Status: newassigned

Hi @tkent, thanks for your explanation and proposed patch!

Do you have time to submit your patch as a pull request? And do you have a CLA on file? https://github.com/dojo/dojo/blob/master/CONTRIBUTING.md goes through some of the steps to make contributing as efficient as possible.

comment:3 Changed 6 years ago by Terence Kent

Hey @Bill

I'd be happy submit my patch as a pull request. I'll do that tomorrow if I have time, or over the weekend if not.

I don't have a CLA on file, but I'll follow the process outlined here to get one: http://dojofoundation.org/about/cla

Thanks, Terence

Last edited 6 years ago by Terence Kent (previous) (diff)

comment:4 Changed 6 years ago by bill

Cool, thanks. The other that needs to be done (hopefully by you) is to add an automated test case for this. We have a bunch of validation tests in dijit/tests/form/robot/ValidationTextBox.html, but apparently not for this case. There's also validationMessages.html. I forget why the tests are spread across two files.

comment:5 Changed 6 years ago by Terence Kent

Bill,

I've sent a pull request with my patch and some unit tests for your review.

After thinking about the problem for a while, I changed the implementation of my solution. The new solution is functionally equivalent, but it's very different from the jsfiddle patch I originally included.

In case you have feedback on the patch, I've joined the contributors mailing list (though have been approved by the moderator yet)

comment:6 Changed 6 years ago by dylan

Milestone: tbd1.10
Priority: undecidedhigh
Summary: NumberTextBox does not properly support leading zeros in editOptions.pattern[patch][cla] NumberTextBox does not properly support leading zeros in editOptions.pattern

comment:7 Changed 6 years ago by bill

See https://github.com/dojo/dijit/pull/46.

Thanks @tkent. We usually add feedback to the PR (pull request), and I've done that.

comment:8 Changed 6 years ago by Bill Keese <bill@…>

Resolution: fixed
Status: assignedclosed

In 9094e1f8fd1202f28241f471cb16c13b7ca29798/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.