Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#14918 closed defect (fixed)

Menu: does not select on first touch (iOS)

Reported by: Pete Smith Owned by: bill
Priority: high Milestone: 1.8
Component: Dijit Version: 1.7.2
Keywords: Cc: ykami
Blocked By: Blocking:

Description

Even on the demo page, on my iPad, if I touch the select (aka click) it opens. If I touch an option, it highlights, but does not select. If I touch it again, it selects.

Attachments (1)

menuFocus.patch (6.4 KB) - added by bill 8 years ago.
Start of patch to not focus menu items on hover or click, but just on keyboard interactions. Need to still remove references to focusedChild from Menu code (ex: in _openPopup()). The dijitMenuItemSelected class no longer gets applied on hover, and dijitSelectMenuSelected now always reflects the current value of the Select, rather than the hovered node which the user has not (yet) clicked to select.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by Douglas Hays

Milestone: tbd1.8
Status: newassigned
Summary: Dijit.form.Select does not respond to touchDijit.form.Select does not select on first touch

comment:2 Changed 8 years ago by Pete Smith

I have patched this on my install, and it works near 100%. At least now it works on iPad.

in Select.js (my class which overrides d.form.Select)

	_fillContent: function(){

			this.inherited(arguments);
			this.dropDown.autoFocus = false; /* I SET THIS TO FALSE. This stopped the first item from being selected */
		}

then in MenuItem?.js line 126:

		focus: function(){
			// summary:
			//		Focus on this MenuItem
			try{
				if(has("ie") == 8){
					// needed for IE8 which won't scroll TR tags into view on focus yet calling scrollIntoView creates flicker (#10275)
					this.containerNode.focus();
				}
				//this.focusNode.focus(); /* NOTE PETE COMMENTED THIS TO FIX IPAD Touch! I don't see how it helps */
			}catch(e){
				// this throws on IE (at least) in some scenarios
			}
		}

This lets the menu work right on iPad, and it works fine on my Firefox desktop as well. If there is one minor grief, there is a subtle little hesitation after I select the menu item on iPad. Some focus issue, but it is so small I don't think people would feel it. I bet with this you can make it perfect. Thank you Doughays.

comment:3 Changed 8 years ago by Douglas Hays

Owner: changed from Douglas Hays to bill
Priority: undecidedhigh

_OnDijitClickMixin is listening for click events and on the first touch, only a touch start/end fire.

comment:4 Changed 8 years ago by bill

Cc: ykami added
Component: Dijit - FormDijit
Summary: Dijit.form.Select does not select on first touchMenu: does not select on first touch

That's a weird one. Usually touching a node causes a click event (ex: dijit/form/Button), but clicking a drop down menu from a dijit/form/Select, dijit/form/ComboButton, or dijit/MenuBar has this issue.

As @httpete showed (thanks!) it's something to do with the focus changes, and I suspect it's a webkit bug. I added some logging and found:

  1. Clicking the Select or DropDownButton moves focus to the HTMTableRowElement aka MenuItem. It's weird to put focus on a TR rather than a TD, although that might be a red herring.
  2. The first (unsuccessful) attempt to click the MenuItem has a touchstart and touchend event, but no click event. After the touchend, there are two spurious focus changes from HTMTableRowElement --> null an then back to the HTMTableRowElement again.
  3. The second click attempt generates a touchstart/touchend and then a click event

I'm cc'ing @ykami because I remember the dojox mobile code changed it's click handling recently, to monitor touchstart/touchend instead of click and IIRC to not treat touchstart --> touchmove --> touchend as a click event. Can you tell me where that code is and why you needed to add it?

PS: @httpete, the reason that focus code is there to begin with is for accessibility reasons, specifically because a screenreader reads whatever is focused. I'm talking about Windows, I'm not sure how/if that works on iPad. But also, see #10716.

Version 1, edited 8 years ago by bill (previous) (next) (diff)

comment:5 Changed 8 years ago by bill

In [28210]:

Don't focus the first menu item when menu drop down is opened by a mouse click. Continue to focus the first menu item if menu is opened via the keyboard. Also, continue to focus the selected item in the dropdown list from a dijit.form.Select. Works around iOS problem on DropDownButton, ComboButton, and Menu/MenuBar where the first click on the drop down menu is ignored.

Refs #14918, fixes #10716 !strict.

comment:6 Changed 8 years ago by bill

For dijit/form/Select, if I set dijit/form/Select.js::_SelectMenu's autoFocus to false, or alternately do the fix listed in this ticket's description, it cures the problem temporarily. I can open a Select, click on a drop down item, and it's selected. But if I repeat (without refreshing the page) I get the same problem again, where the first click is ignored.

It seems like mobile safari just doesn't like it when you programatically muck with focus. Will investigate more.

comment:7 Changed 8 years ago by bill

Summary: Menu: does not select on first touchMenu: does not select on first touch (iOS)

comment:8 Changed 8 years ago by bill

I looked at eliminating/reducing focus events during mouse/touch operations. Focusing a MenuItem on hover is strange, as is focusing a MenuItem on click, unless it's on a *non*-popup menu, such as a MenuBar, in which case it's debatable.

I think at some point those extraneous focus events should be removed, but not sure if it would fully solve this problem since we still want to, for example, focus the first <input> upon opening a TooltipDialog, and then arguably, on closing the TooltipDialog, focus back to the <button> that opened it.

So I've found a simple workaround to this problem that I'll check in now, and leave rearchitecture to a later time.

comment:9 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

In [28275]:

Manually generate click event from touchend event to avoid iOS/safari bugs when a click event doesn't occur after focus has been programatically shifted.

Right now the code is just listening for touchend events. dojox/mobile is doing something more complicated where it listens to touchstart, touchmove, and touchend. That's not needed on most dijit pages since touchmove just scrolls the document, while keeping the finger on the originally touched node. But maybe dijit will need to do it eventually for pages that trap touchmove and call evt.preventDefault(), thus allowing a user to touchstart somewhere random, slide to the node with the a11yclick handler, and then do a touchend.

Fixes #14918, refs #14604 !strict.

Changed 8 years ago by bill

Attachment: menuFocus.patch added

Start of patch to not focus menu items on hover or click, but just on keyboard interactions. Need to still remove references to focusedChild from Menu code (ex: in _openPopup()). The dijitMenuItemSelected class no longer gets applied on hover, and dijitSelectMenuSelected now always reflects the current value of the Select, rather than the hovered node which the user has not (yet) clicked to select.

comment:5 Changed 7 years ago by bill

In [28853]:

Don't fire synthetic click event until after all touchend listeners have finished running.

This fixes the problem where opening a DropDownButton's drop down on iOS would open then immediately close. _OnDijitClickMixin was firing the synthetic click event too early, which was flummoxing _HasDropDown because _onDropDownClick() was getting called before _onDropDownMouseUp().

Fixes #15501, refs #14918 !strict.

Note: See TracTickets for help on using tickets.