Opened 9 years ago
Closed 9 years ago
#15859 closed defect (fixed)
ListItem doesn't highlight for "quick" touches
Reported by: | Jeremiah Orr | Owned by: | Eric Durocher |
---|---|---|---|
Priority: | high | Milestone: | 1.8.1 |
Component: | DojoX Mobile | Version: | 1.8.0 |
Keywords: | Cc: | cjolif, Adrian Vasiliu | |
Blocked By: | Blocking: |
Description
As of 1.8.0, if you click/touch a dojox.mobile.ListItem? relatively quickly, it will perform the transition, but will not highlight the button. If you click/touch and hold a little longer, it will highlight correctly.
The attached file shows this behavior. If you switch to 1.7.3, it highlights even with quick clicks/touches.
I have observed this on iOS Safari 5.1.1 and Mac OS X Chrome 21.
Attachments (3)
Change History (14)
Changed 9 years ago by
Attachment: | touchTest.html added |
---|
Changed 9 years ago by
Attachment: | patch15859.patch added |
---|
comment:1 Changed 9 years ago by
Right. This is a side-effect of changeset 27598 for dojox/trunk/mobile/_ItemBase.js, which introduced the "delayed selection" mechanism, to avoid the unwanted selection of items during scrolling.
A workaround is to set item._delayedSelection at false (in markup, by adding "_delayedSelection:false" to the data-dojo-props of each item).
Attached a candidate patch, which aims to preserve the "delayed selection" mechanism while ensuring the item is selected before transitioning (it is deselected after transition).
Tested this patch with ListItem and IconMenu (another subclass of _ItemBase), on desktop (Chrome, Firefox) and mobile (iOS and Android).
comment:2 Changed 9 years ago by
Milestone: | tbd → 1.8.1 |
---|---|
Priority: | undecided → high |
comment:3 Changed 9 years ago by
Cc: | cjolif added |
---|
comment:4 Changed 9 years ago by
The problem I see with the patch is that is there is a user click action it won't call the defaultClickAction and thus don't see the selected state on the button. I'm not sure this is expected? Why not doing the selected call in onTouchEnd instead?
comment:5 Changed 9 years ago by
Cc: | Adrian Vasiliu added |
---|
Changed 9 years ago by
Attachment: | patch15859-variant-handleSelection.patch added |
---|
Variant of the fix for the "delayedSelection": now the item is selected before transitioning - Adrian Vasiliu, IBM, CCLA
comment:6 Changed 9 years ago by
I'm not sure calling it in _ItemBase._onTouchEnd would be better. In this method, we don't know yet what will happen next. This method delegates the handling of the event to _onClick, which is *not* defined on _ItemBase, instead it is defined and implemented in the subclasses (ListItem, IconMenu). These implementations of _onClick in the subclasses are the ones which call either the user-defined onClick, or the defaultClickAction. It may not be okay to assume that any user-defined onClick would always like this selection to happen. Also, a user-defined onClick, if it wants the selection to be handled similar to the default, has anyway to call by itself _ItemBase.handleSelection, since this is only called by defaultClickAction.
Given that defaultClickAction is already the only one to call handleSelection, it looked consistent to me it's also this method which takes care of this additional selection. But, all in one, I would rather propose to move the call from defaultClickAction to _ItemBase.handleSelection, as in the attached patch15859-variant-handleSelection.patch. This is technically equivalent to the previous variant, but probably semantically better, grouping together selection-related stuff. This way, for a user-defined click action, just calling handleSelection would restore the entire default behavior for the selection (if needed).
comment:7 Changed 9 years ago by
Adrian, can you check something? There is some code in _onTouchMove that is here to "cancel" the selection if the user moves touch during the selection gesture. I fear that your patch might override that behavior. Can you investigate? Thanks.
comment:8 Changed 9 years ago by
ok found the answer myself, _onTouchMove calls cancel() which removes the touch end listener and so the added code is never called in that case and thus can't cause an issue.
That said even if my code is maybe not "better" what is sure is that it would better correspond to the behavior that existed before the delayed selection mechanism was introduced and would be more consistent between when this mode is one vs not. Actually if the delay is 0, you will see the selection happens whatever is the user-defined onClick method while with your solution if the user-defined onClick returns false then no selection will occur when there is a delayed selection vs one will occur when there is not. That said the right solution is probably wider than that because there are other side problems like the fact the selection is partly handled in always executed code, partly handled in code that is only executed if user-defined onClick returns true. So waiting for the "right" solution managing onClick with false let's do what you propose that is fixing the issue in most cases and where the only potential problem is with a case that anyway needs some rework.
comment:11 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fix for the "delayedSelection": now the item is selected before transitioning - Adrian Vasiliu, IBM, CCLA