Opened 12 years ago

Closed 9 years ago

Last modified 9 years ago

#6416 closed defect (fixed)

_DateTimeTextBox: better a11y

Reported by: Nathan Toone Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit - Form Version: 1.1.0
Keywords: Cc: bill, Becky Gibson
Blocked By: Blocking:

Description (last modified by bill)

Both DateTextBox and TimeTextBox should pass certain keypress events through to the underlying widget. This would actually be a change to _DateTimeTextBox.

The calendar would support as many shortcuts from http://dev.aol.com/dhtml_style_guide as would be feasably possible (without shifting focus to the underlying calendar widget).

The time picker would support up/down, and possibly others (pageup/pagedown for per-hour moves, etc).

Attachments (1)

TimePicker_upDown-6416-trunk-2008-07-04_0845.diff (3.8 KB) - added by nathan 12 years ago.
First stab at a patch with allows keyboard shortcuts to navigate the time picker. This also is a good starting point to have calendar navagable as well.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 12 years ago by bill

Summary: Better a11y for dijit._TimePickerTimeTextBox: better a11y

comment:2 Changed 12 years ago by nathan

Here is some code from gearb0x on IRC to handle this for TimeTextBox?. Probably needs some work - and might be good to move into _TimePicker instead.

dojo.declare("UpDownTimeTextBox",dijit.form.TimeTextBox,{
    _onKeyPress: function(evt) {
	if(evt.keyCode == dojo.keys.UP_ARROW)
	{
	    var picker = this._picker;

	    if(this.textbox.value == '')
	    {
		// Pick the first item
		var selectedItm = picker.timeMenu.childNodes[0];

		var tdate = selectedItm.date || selectedItm.parentNode.date;
		if(!tdate || picker.isDisabledDate(tdate)){ return; }
		picker.value = tdate;
		dijit.form.TimeTextBox.superclass.setValue.call(this, tdate, true);

		var selectedIndex = null
		var selectedItm = null;

		// Find the selected div
		for(var i = 0;i<picker.timeMenu.childNodes.length;i++)
		{
		    var sdate = picker.timeMenu.childNodes[i].date || picker.timeMenu.childNodes[i].parentNode.date;

		    if(sdate.toString() == tdate.toString())
		    {
			selectedIndex = i;
			selectedItm = picker.timeMenu.childNodes[i];
		    }
		}
console.debug(selectedIndex);
		picker.timeMenu.childNodes[selectedIndex].selected = true;
		
   		dojo.removeClass(picker.timeMenu.childNodes[selectedIndex], picker.baseClass+"ItemSelected");
		dojo.addClass(picker.timeMenu.childNodes[selectedIndex], picker.baseClass+"ItemHover");

	    }
	    else
	    {
		// we allready have a selected item
		// find selected class
		var selectedIndex = null
		var selectedItm = null;

		for(var i = 0;i<picker.timeMenu.childNodes.length;i++)
		{
		    
		    if(picker.timeMenu.childNodes[i].date.getHours() == this.value.getHours() && picker.timeMenu.childNodes[i].date.getMinutes() == this.value.getMinutes())
		    {

			selectedIndex = i;
			selectedItm = picker.timeMenu.childNodes[i];
		    }
		}

		// De Select it
		picker.timeMenu.childNodes[selectedIndex].selected = false;
		dojo.removeClass(picker.timeMenu.childNodes[selectedIndex-1], picker.baseClass+"ItemHover");

		// Select the next
		selectedItm = picker.timeMenu.childNodes[selectedIndex-1];

		var tdate = selectedItm.date || selectedItm.parentNode.date;
		if(!tdate || picker.isDisabledDate(tdate)){ return; }
		picker.value = tdate;
		dijit.form.TimeTextBox.superclass.setValue.call(this, tdate, true);

		// find the node again (as the arry can get redone from the above call)
		for(var i = 0;i<picker.timeMenu.childNodes.length;i++)
		{
    		    if(picker.timeMenu.childNodes[i].date.getHours() == this.value.getHours() && picker.timeMenu.childNodes[i].date.getMinutes() == this.value.getMinutes())
		    {
			selectedIndex = i;
			selectedItm = picker.timeMenu.childNodes[i];
		    }
		}

		picker.timeMenu.childNodes[selectedIndex].selected = true;
		
		dojo.removeClass(picker.timeMenu.childNodes[selectedIndex], picker.baseClass+"ItemSelected");
		dojo.addClass(picker.timeMenu.childNodes[selectedIndex], picker.baseClass+"ItemHover");

	    }
	}
	else if(evt.keyCode == dojo.keys.DOWN_ARROW)
	{
	    var picker = this._picker;

	    if(this.textbox.value == '')
	    {
		// Pick the first item
		var selectedItm = picker.timeMenu.childNodes[0];

		var tdate = selectedItm.date || selectedItm.parentNode.date;
		if(!tdate || picker.isDisabledDate(tdate)){ return; }
		picker.value = tdate;
		dijit.form.TimeTextBox.superclass.setValue.call(this, tdate, true);

		var selectedIndex = null
		var selectedItm = null;

		// Find the selected div
		for(var i = 0;i<picker.timeMenu.childNodes.length;i++)
		{
		    var sdate = picker.timeMenu.childNodes[i].date || picker.timeMenu.childNodes[i].parentNode.date;

		    if(sdate.toString() == tdate.toString())
		    {
			selectedIndex = i;
			selectedItm = picker.timeMenu.childNodes[i];
		    }
		}
console.debug(selectedIndex);
		picker.timeMenu.childNodes[selectedIndex].selected = true;
		
   		dojo.removeClass(picker.timeMenu.childNodes[selectedIndex], picker.baseClass+"ItemSelected");
		dojo.addClass(picker.timeMenu.childNodes[selectedIndex], picker.baseClass+"ItemHover");

	    }
	    else
	    {
		// we allready have a selected item
		// find selected class
		var selectedIndex = null
		var selectedItm = null;

		for(var i = 0;i<picker.timeMenu.childNodes.length;i++)
		{
		    
		    if(picker.timeMenu.childNodes[i].date.getHours() == this.value.getHours() && picker.timeMenu.childNodes[i].date.getMinutes() == this.value.getMinutes())
		    {

			selectedIndex = i;
			selectedItm = picker.timeMenu.childNodes[i];
		    }
		}

		// De Select it
		picker.timeMenu.childNodes[selectedIndex].selected = false;
		dojo.removeClass(picker.timeMenu.childNodes[selectedIndex+1], picker.baseClass+"ItemHover");

		// Select the next
		selectedItm = picker.timeMenu.childNodes[selectedIndex+1];

		var tdate = selectedItm.date || selectedItm.parentNode.date;
		if(!tdate || picker.isDisabledDate(tdate)){ return; }
		picker.value = tdate;
		dijit.form.TimeTextBox.superclass.setValue.call(this, tdate, true);

		// find the node again (as the arry can get redone from the above call)
		for(var i = 0;i<picker.timeMenu.childNodes.length;i++)
		{
    		    if(picker.timeMenu.childNodes[i].date.getHours() == this.value.getHours() && picker.timeMenu.childNodes[i].date.getMinutes() == this.value.getMinutes())
		    {
			selectedIndex = i;
			selectedItm = picker.timeMenu.childNodes[i];
		    }
		}

		picker.timeMenu.childNodes[selectedIndex].selected = true;
		
		dojo.removeClass(picker.timeMenu.childNodes[selectedIndex], picker.baseClass+"ItemSelected");
		dojo.addClass(picker.timeMenu.childNodes[selectedIndex], picker.baseClass+"ItemHover");

	    }

	}
	else
	{
	    this.inherited(arguments);
	}
    }
});

comment:3 Changed 12 years ago by nathan

Status: newassigned

comment:4 Changed 12 years ago by James Burke

Just make sure that if you use this patch, you can verify who gearb0x is and that a CLA is signed for that person.

Changed 12 years ago by nathan

First stab at a patch with allows keyboard shortcuts to navigate the time picker. This also is a good starting point to have calendar navagable as well.

comment:5 Changed 12 years ago by Becky Gibson

Cc: Becky Gibson added

comment:6 Changed 12 years ago by nathan

Just as a clarification - the patch that is attached is *not* based on what was sent in. It was entirely created by me from scratch. gearb0x just inspired me to get something done with this.

comment:7 Changed 12 years ago by nathan

Description: modified (diff)
Summary: TimeTextBox: better a11y_DateTimeTextBox: better a11y

comment:8 Changed 12 years ago by nathan

We still need to figure out a strategy for left/right on the calendar....unless we shift focus, we want to keep left/right for controlling the cursor position in the text box...

comment:9 Changed 12 years ago by Becky Gibson

This is a great start! Bill, are you comfortable with the api changes since it is to "private" classes? There are a few UI and usability issues. When I mouse over a quarter hour time it is made large enough to see - the same thing needs to happen when I use the arrow key to focus one of them. currently the font is too small to see. And, it seems kind of odd that focus stays in the same area and doesn't really move up and down in the time picker - rather the time moves to stay in the same place (not sure If I explained that well enough). It would be good to get some feedback from the usability folks. But, this at least gives us keyboard support in the timepicker. Bill, should we check in?

comment:10 Changed 12 years ago by nathan

I'm willing to make changes to it, if needed.

As for the focus staying in the same place, that's a side-effect of how I had implemented it. Perhaps, what I should do, instead of actually "Changing" the value (like I currently do...notice as you move up and down it actually updates the text box field), I should mimic a "hover" of the mouse. That would fix both the focus positioning and the zooming (readability) issue - and is probably the better way to go.

comment:11 Changed 12 years ago by nathan

Actually - in thinking about it a bit more, the "hover-based" way of doing it (rather than "select-based") is probably the way to go. That would also avoid the change to the onValueSelected event.

As for changes to the API - there is some sort of underlying need for a "handleKey" function - I don't know if that should be private or public...but the existence of such a function would be a key to the _DateTimeTextBox so it knows to pass the event to the underlying widget.

And as far as I understand, the only reason _Calendar and _TimePicker are private is because they aren't accessible right now...is that not correct?

comment:12 Changed 12 years ago by nathan

Still thinking about this one.... :)

I'm wondering if this ticket should just be for the addition of passing in the keyboard event to handleKey (if the function exists)...then separate issues could be raised to implement handleKey in _TimePicker and _Calendar...

Then it might be easier to track the individual issues...(but at least the API and behavior would be defined).

The suggested API would be, for any widget that is used as the picker for a dijit.form._DateTimeTextBox, it *MAY* implement a handleKey function - which takes a keyboard event object (no mouse objects). This function *MAY* cancel the event, if needed - and can completely ignore it if a key is passed that is not interesting to that particular widget.

If going this route, the only changes to implement this API on the text box side would be the two additions (technically, just the first one - the second is a bugfix that I slipped in...) to the _onKeyPress function in dijit/form/_DateTimeTextBox.js. Implementations of handleKey could then be tracked separately.

thoughts?

comment:13 Changed 12 years ago by guest

gearb0x here

give the code i pasted a go, it simulates a hover rather than selected, and moves the selected item down ~4 times before it shifts the times

Might be able to put the idea into your diff, although I'm not sure what does the actual scroling i think its the call: dijit.form.TimeTextBox?.superclass.setValue.call(this, tdate, true);

Also have a Signed CLA now :p Not that my code is much good heh

comment:14 Changed 12 years ago by guest

select based is fine so it updates the text box every time you press up and down so when you tab out the values saved ready to go :D

comment:15 Changed 12 years ago by bill

Regarding Becky's comment I'm fine w/the code changes, but agree w/Becky's comments about the UI. See the forum post for discussion about whether keyboard focus and current selection should be unified or independent but my feeling is that they should be unified (like the comment prior to this one).

comment:16 Changed 12 years ago by Nathan Toone

(In [13456]) fixes #5639, refs #6416: added filtering to the timepicker and up/down arrow support for the time picker as well. also some minor theme fixes !strict

comment:17 Changed 11 years ago by nathan

Milestone: 1.21.3
Status: assignednew

TimeTextBox? has a11y (keyboard) support - moving to 1.3 for DateTextBox? keyboard support

comment:18 Changed 11 years ago by Nathan Toone

Owner: changed from nathan to Nathan Toone

comment:19 Changed 11 years ago by Nathan Toone

Reporter: changed from nathan to Nathan Toone

comment:20 Changed 11 years ago by bill

Milestone: 1.31.5

comment:21 Changed 10 years ago by Nathan Toone

Milestone: 1.5future

comment:22 Changed 9 years ago by Nathan Toone

Owner: Nathan Toone deleted

Unassigning my tickets.

comment:23 Changed 9 years ago by bill

Milestone: future1.6
Owner: set to bill
Status: newassigned

I'm updating DateTextBox to extend _HasDropDown now (#9945), so I might as well do this ticket right afterwards.

As for the left/right arrow keys, they will adjust the Calendar while it's open, so the user will have to close the drop down to use the arrow keys on the text box. But I'm also fixing #5151 so that focusing a widget won't automatically open the drop down, so I don't think that will be an issue.

comment:24 Changed 9 years ago by bill

Description: modified (diff)

(In [22556]) Fix Calendar to handle keypress events forwarded from DateTextBox, thus making the DateTextBox's calendar drop down keyboard accessible.

Remaining issues include:

  • Needs review for screen reader support (if that's possible to support). One possible modification is to make every keystroke change the value in the <input> field, so that the screen reader knows what happened.
  • Also, need to add automated tests (after the a11y review).
  • ENTER key handling: in Calendar, every arrow key press changes the Calendar's value, firing onChange(). Note the difference from ColorPalette, which only fires onChange() upon the ENTER/SPACE key. The difference is because ColorPalette cells gets focus and using the arrow keys just changes the focused cell, rather than changing the value... whereas Calendar as a drop down doesn't focus at all.

Normally onChange() makes dijit.popup() report an onExecute event, which then makes _HasDropDown close the drop down. Working around this now by adding an explicit onExecute() method to Calendar which is only called on the ENTER/SPACE key. But I only intended the onExecute() callback to be used for fancy drop downs like TooltipDialog. Maybe there's a better way to handle this, perhaps flagging Calendar as an always-fires-onchange type widget and monitoring for ENTER key in _HasDropDown itself.

  • Fix key repeat. Typematic never worked for TimeTextBox because of complications with forwarding the key events from the TimeTextBox to the _TimePicker. However, key repeat did work because we were attaching to the onkeypress event. _HasDropDown currently attaches to onkeyup rather than onkeypress. Switching it to onkeypress is not trivial, because _HasDropDown also has an onkeydown handler which calls evt.preventDefault(), which on IE makes the onkeypress event not fire for ENTER/SPACE key. (In other words, need to be careful that we don't break ENTER/SPACE key handling.)

comment:25 Changed 9 years ago by bill

Resolution: fixed
Status: assignedclosed

I'm closing this long ticket and tracking further changes for Calendar navigation in #9918. Note that _TimeTextBox has supported keyboard navigation for some time now.

comment:26 Changed 9 years ago by bill

(In [22580]) Various [stop gap] fixes to TimeTextBox, although still wondering about full rewrite as per #7631, or replacing custom scrolling with native scrollbar like ComboBox has. Fixes include:

  • limit drop down to 10 entries so that (in the majority of cases) it won't overlap the <input> box (fixes #8387)
  • same as ComboBox, make typing automatically open the dropdown, and once the user has started typing or backspacing the dropdown list is filtered by what's in the <input> (fixes #11486)
  • fix recently introduced problem (with _HasDropDown conversion of _DateTimeTextBox, refs #6416) where drop down becomes detached from <input> when it appears above the input and then user types text to further filter the values shown
  • add more tests for TimeTextBox; they previous tests were very sparse; tests include check of current filtering sort order (refs #11485)

!strict as usual.

comment:27 Changed 9 years ago by bill

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