Opened 13 years ago

Closed 13 years ago

#2046 closed defect (wontfix)

DropdownTimeTimePicker/TimePicker misbehavior with onValueChange

Reported by: kevin@… Owned by: tk
Priority: high Milestone: 0.9
Component: Widgets Version: 0.4
Keywords: Cc:
Blocked By: Blocking:

Description

I've been advised that there may be CLA concerns. I am an independent developer. I am not employed by anybody. My work belongs to me. I can do whatever to document this, if you think there is any merit to this bug report.

When the value of the dojo.TimePicker? is set via TimePicker?.setTime(), as it is by the DropdownTimePicker? in DropdownTimePicker?.setTime(), TimePicker?.initUI is called to reflect the value in the UI and to set internal properties. This routine does this by calling the same value setters invoked by the UI. Unfortunately, this results in the rapid generation of 3 onValueChange events. The values associated with the first two events are intermediate results and are likely incorrect. This is a scenario that is normal for a user of the UI, but happens at a much slower pace, allowing the user to react to the fact that using the widget may result in incorrect intermediate values as the user specifies the hour, minute and am/pm settings. When this happens programmatically, the events are so clustered that incorrect values can find their way into databases via asynchrous xmlhttp calls.

When TimePicker?.setTime is used to set the value of the widget, only one onValueChange event should be generated. This is initutively correct and enhances the usefulness of the widget.

This is easily accomplished by making the following changes to TimePicker?.initUI. I've simply replaced the setter calls with the contents of the setters and commented out unnecessary duplicate calls. This solution maintains the internal integrity of the widget and is simply as modest refactoring of a single method.

initUI: function() { set UI to match the currently selected time if(!this.selectedTime.anyTime && this.time) { var amPmHour = dojo.widget.TimePicker?.util.toAmPmHour(this.time.getHours()); var hour = amPmHour[0]; var isAm = amPmHour[1]; var minute = this.time.getMinutes(); var minuteIndex = parseInt(minute/5); this.onSetSelectedHour(this.hourIndexMap[hour]); this.onClearSelectedAnyTime(); this.onClearSelectedHour(); this.setSelectedHour(this.hourIndexMap[hour]); this.onSetTime(); this.onSetSelectedMinute(this.minuteIndexMap[minuteIndex]); this.onClearSelectedAnyTime(); this.onClearSelectedMinute(); this.setSelectedMinute(this.minuteIndexMap[minuteIndex]); this.selectedTime.anyTime = false; this.onSetTime(); this.onSetSelectedAmPm(isAm); this.onClearSelectedAnyTime(); this.onClearSelectedAmPm(); this.setSelectedAmPm(isAm); this.selectedTime.anyTime = false; this.onSetTime(); } else { this.onSetSelectedAnyTime(); }

Attachments (1)

TimePicker.patch (980 bytes) - added by guest 13 years ago.
TimePicker? patch file

Download all attachments as: .zip

Change History (4)

comment:1 Changed 13 years ago by bill

Kevin, sounds like a valid bug. The CLA thing is http://www.dojotoolkit.org/icla.txt (you have to send it in). Also, you should attach patches (not inlining them into the bug report, since the formatting gets messed up), and put them in patch file format (generated from eclipse, tortoisesvn, etc.)

Changed 13 years ago by guest

Attachment: TimePicker.patch added

TimePicker? patch file

comment:2 Changed 13 years ago by tk

Owner: changed from Karl Tiedt to tk

comment:3 Changed 13 years ago by bill

Resolution: wontfix
Status: newclosed

TimePicker? is being abandoned but we will build a new TimePicker? at some point. Separate bug is filed for that.

Note: See TracTickets for help on using tickets.