Opened 9 years ago

Closed 7 years ago

#5842 closed bug (duplicate)

Does not update field on date select

Reported by: igor.nadj@… Owned by:
Priority: minor Milestone: 1.11.0
Component: ui.datepicker Version: 1.8.2
Keywords: Cc:
Blocked by: Blocking:

Description

(Little bit of a bad title, rather it reselects the current date instead of select the hovered date)

Replication:

  1. select a date (any method)
  2. reopen the datepicker, use ctrl-arrow keys to select a different date. Note: the new date must be before the previously selected date. Press enter.

Expected output: field changes to new date. Actual output: field does not change to new date.

Problem code:

...
_doKeyDown: function(event) {
...
case 13: var sel = $('td.' + $.datepicker._dayOverClass, inst.dpDiv).add($('td.' + $.datepicker._currentClass, inst.dpDiv));

Bug info: The intended behaviour here when enter is pressed is to update the state and field with the newly selected date.

It builds an array, the first element is the dayOverClass, the newly selected date. The second element I believe is the fallback, the current date.

However, the method used to build this collection, the add() function, does not guarantee that new elements will be appended to the end of the collection. In fact, it uses DOM ordering. Therefore, if your newly selected date is after the old date, you're fine, but if it's before, the order of the collection is backwards and the code uses the wrong date.

Code fix: Replace:

case 13: var sel = $('td.' + $.datepicker._dayOverClass, inst.dpDiv).add($('td.' + $.datepicker._currentClass, inst.dpDiv));

With:

case 13: var sel = [$('td.' + $.datepicker._dayOverClass, inst.dpDiv), $('td.' + $.datepicker._currentClass, inst.dpDiv)];

Change History (7)

comment:1 Changed 9 years ago by Jörn Zaefferer

Component: ui.coreui.datepicker

comment:2 Changed 9 years ago by jamescryer

I have found the same problem. The existing code doesn't not work because the DOM order is not as the code expects i.e., sel[0] is not always the selected date td.

My fix is:

var sel = $('td.' + $.datepicker._dayOverClass, inst.dpDiv); sel = sel.length > 0 ? sel : $('td.' + $.datepicker._currentClass, inst.dpDiv);

comment:3 Changed 9 years ago by jamescryer

The bug can be reproduced on http://jqueryui.com/demos/datepicker/.

1) Select a date
2) Gain focus and open the datepicker by pressing enter
3) Use the ctrl+right key to move to a future day on the current month
4) Press enter.

The date field is populated with the current date not the selected date.

Please increase the priority of this bug, this is obviously a serious issue with regard to accessibility.

comment:5 Changed 9 years ago by goobsoft

I'd like to see this fixed. I love the keyboard :)

I don't like using "add" to create a set only to use the first element. What element are you going to get out of the set first? It seems to not always be the first one you put in which makes sense for a set.

Here is the solution I came up with before I found this bug report:

case 13:						
	var sel = $('td.' + $.datepicker._dayOverClass, inst.dpDiv)
	if (!sel)
		sel = $('td.' + $.datepicker._currentClass, inst.dpDiv)
	if (sel)
		$.datepicker._selectDay(event.target, inst.selectedMonth, inst.selectedYear, sel);
	else 
		$.datepicker._hideDatepicker();

comment:6 Changed 7 years ago by Scott González

Milestone: TBD1.11.0

comment:7 Changed 7 years ago by mikesherov

Resolution: duplicate
Status: newclosed

Duplicate of #5527.

Note: See TracTickets for help on using tickets.