Opened 7 years ago
Closed 7 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 7 years ago by
comment:2 Changed 7 years ago by
Owner: | set to bill |
---|---|
Status: | new → assigned |
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 7 years ago by
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
comment:4 Changed 7 years ago by
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 7 years ago by
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 7 years ago by
Milestone: | tbd → 1.10 |
---|---|
Priority: | undecided → high |
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 7 years ago by
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 7 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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/