Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#10822 closed enhancement (fixed)

dijit.form._DateTimeTextBox accepts a default value for the UI when the field value is null

Reported by: mattkime Owned by: Douglas Hays
Priority: high Milestone: 1.6
Component: Dijit - Form Version: 1.4.0
Keywords: DateTextBox Cc: Douglas Hays
Blocked By: Blocking:

Description (last modified by bill)

If there is no value in the field, a DateTimeTextBox UI defaults to the current date/time. This allows you to set a default value for the UI to present.

Attached is a patch file with both code and test.

Attachments (1)

dateTextbox_defaultValue.txt (4.8 KB) - added by mattkime 10 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by Adam Peller

Cc: Douglas Hays added
Milestone: 1.4.2tbd

Looks simple enough, though I'm concerned about adding yet more complexity about the concept of 'empty' in these widgets

comment:2 in reply to:  1 Changed 10 years ago by mattkime

Replying to peller:

Looks simple enough, though I'm concerned about adding yet more complexity about the concept of 'empty' in these widgets

I'm happy to have that debate. can you be more specific about your concerns? This fix only aims to provide a better UI default value that now when the field is empty.

comment:3 Changed 10 years ago by Douglas Hays

Thanks for the proposed patch. The patch ignores the case when the Calendar popup is already open and the DateTextBox? value is programmatically set to null. In this case, the Calendar is set to "now" and not the new default. Also, the defaultValue is specified as
defaultValue="new Date(2010,0,1)" and it should be defaultValue="2010-01-01" in markup for consistency which doesn't work with the supplied patch. The patch needs to also enhance dijit/tests/form/robot/DateTextBox.html to create a new automatic testcase for regression testing demonstrating the new function.

comment:4 Changed 10 years ago by mattkime

The patch ignores the case when the Calendar popup is already open

Can you point to an example of this? I'm unfamiliar with this use case.

I'm working on addressing the data format and robot test issues.

Changed 10 years ago by mattkime

comment:5 Changed 10 years ago by mattkime

fixed the following issues - Setting field to null while calendar popup is open Markup now takes yyyy-mm-dd format added robot tests

comment:6 Changed 10 years ago by mattkime

let me try that again -

fixed the following issues -
Setting field to null while calendar popup is open
Markup now takes yyyy-mm-dd format
added robot tests

comment:7 Changed 10 years ago by dante

Owner: set to Douglas Hays

comment:8 Changed 10 years ago by Douglas Hays

Matt, you changed

this._picker.set('value', this.get('value') || new this.dateClassObj());

to

this._picker.set('value', this.get('value') || this.get('defaultValue') || new this.dateClassObj());

but the defaultValue will always have a value (defaults to Invalid Date object==NaN) and the current date will never be set via dateClassObj. Did you mean something like this instead:

var defaultValue;
this._picker.set('value',
        this.get('value') || (defaultValue=this.get('defaultValue'), (!defaultValue || isNaN(defaultValue)) ? new this.dateClassObj() : defaultValue));

comment:9 Changed 10 years ago by mattkime

doughays -

you're certainly correct about that problem but i'm confused why it wouldn't also be true of "value". What kind of processing does "value" receive? It seems that "defaultValue" might receive the same.

perhaps it would make more sense to add to postMixInProperties -

if(isNaN(this.defaultValue) ){ this.defaultValue = null; }

I would prefer that defaultValue defaulted to null but then the value won't be processed as a date.

comment:10 Changed 10 years ago by bill

Description: modified (diff)

Why not make the default value of defaultValue be new Date(), rather than new Date("")? That seems like the whole point of this ticket, that the Calendar opens to a specified (valid, non-null) date when the DateTextBox has no value.

There's a minor issue that the Calendar shouldn't show any day as pre-selected, but rather show all the days unhighlighted until the user clicks one. So really we just want to specify defaultValue="2010-06" or something like that. But that's an issue with the current DateTextBox too, regardless of this patch: that it defaults to opening with today pre-selected.

Also, I wish there was a less confusing name than "defaultValue".

comment:11 Changed 10 years ago by Douglas Hays

Milestone: tbd1.6

Deferring to 1.6 to give everyone a chance to find the best solution/API.

comment:12 Changed 9 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [22663]) Fixes #10822. Add dropDownDefaultValue/Date object so that DateTextBox? can set a value other than "now" on the popup GUI when the actual textbox is blank. Added autoamted testcases.

comment:13 Changed 9 years ago by bill

(In [22772]) Modifications to DateTimeTextBox and Calendar so that dropDownDefaultValue controls the Calendar's currentFocus, rather than it's value. Calendar by default now has no selected date, although focus (by default) goes to today's date.

All of this code is complicated by invalid dates sometimes being represented as null, and sometimes as Date(""), the NaN of dates. Hopefully that will be simplified in 2.0.

Refs #11625, #10822 ([22665]), !strict.

comment:14 Changed 9 years ago by bill

Component: DijitDijit - Form
Note: See TracTickets for help on using tickets.