Opened 12 years ago

Closed 12 years ago

Last modified 10 years ago

#3461 closed defect (fixed)

Widget.connect onclick should register some key based triggers.

Reported by: davidb Owned by: davidb
Priority: high Milestone: 0.9beta
Component: Dijit Version: 0.9
Keywords: Cc: koranteng, simonjb, bill
Blocked By: Blocking:

Description

onclick fires for most elements via mouse, but for many of them it can not be triggered via keyboard.

We should have Widget.connect provide key based triggers as/when necessary. Most likely the best keys to use are the space-key and possibly the enter-key. Probably safest to trigger on keyup.

(filed as per dojo-meeting today)

Attachments (5)

3461-kb-click.diff (774 bytes) - added by davidb 12 years ago.
thinking about something like this…
3461-kb-click.2.diff (951 bytes) - added by davidb 12 years ago.
or this (a new keyclick event) to add in templates as/when necessary
3461-kb-click.3.diff (5.7 KB) - added by davidb 12 years ago.
update of work so far. (possible solution)
3461-kb-click.4.diff (6.8 KB) - added by davidb 12 years ago.
a better fix
3461-kb-click.5.diff (8.0 KB) - added by davidb 12 years ago.
I think this is a decent baby step towards consistent kb based click.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 12 years ago by davidb

Cc: koranteng simonjb added

comment:2 Changed 12 years ago by davidb

Owner: changed from bill to davidb

comment:3 Changed 12 years ago by davidb

Status: newassigned

Changed 12 years ago by davidb

Attachment: 3461-kb-click.diff added

thinking about something like this...

comment:4 Changed 12 years ago by davidb

Cc: bill added

The danger with this patch is perhaps if the dijit in question already has a keyboard actionable onclick handler. (e.g. has an html button node that captures the click). One way around this is to make sure e._dijitKeyboardGenerated is checked and the event ignored in those cases. This might be hard to diagnose though ... if folks encounter two clicks and don't know why.

Maybe output a warning console.debug message?

Changed 12 years ago by davidb

Attachment: 3461-kb-click.2.diff added

or this (a new keyclick event) to add in templates as/when necessary

comment:5 Changed 12 years ago by davidb

Update: preparing patch that will affect (slightly): ColorPalette?, TitlePane?, Widget, Button, ComboBox?, InlineEditBox?, popup .js files.

Will need to test before posting/commiting.

(Note, in some places code could be quite simplified if enter also fires click... not sure about enter though).

comment:6 Changed 12 years ago by bill

I thought you would check the type of the DOM node and add the special keyboard handler only if necessary.

Also, this.disconnect needs to be able to undo whatever this.connect did, so this needs some more work. Probably easier with a new name for the synthetic event like in your second patch. (I'd like to think of a better name than onkeyclick but nothing comes to mind. "activate" seems wrong as "active" typically refers to a button while it's being pressed.)

comment:7 Changed 12 years ago by davidb

OK. I'll proceed with the second patch. Perhaps naming the event "trigger" or something similar.

Changed 12 years ago by davidb

Attachment: 3461-kb-click.3.diff added

update of work so far. (possible solution)

Changed 12 years ago by davidb

Attachment: 3461-kb-click.4.diff added

a better fix

comment:8 Changed 12 years ago by davidb

Two solutions have been attempted:

In _Widget.connect...

  1. Add key listening to fire the onclick handler.

in pseudocode:

if (event == "onclick" && node-needs-kb-click) {
 // auto-add keyboard listener that calls
 // click handler when space key up happens.
}
  1. Add new event "onklick".

in pseudocode:

if (event == "onklick" && node-needs-kb-click()) { 
 // auto-add keyboard listener that calls
 // click handler when space key up happens.
}

Conclusion: Avoiding conflict with existing keyboard based "click" triggers is problematic.

  1. node-needs-kb-click is impossible to get right in all cases, since clicks can bubble. It is not just a matter of checking for the current, or for a child node that has kb based click already, but we need to know what the handler does with the event. Does it bubble? What does the author want?
  1. onklick has grown on me and I think it offers several advantages:
  • clarity. "onklick" should tell an author that this does something different, and we are doing something different. We are adding a side-effect (a very handy one) of space/enter key-up connection to the onklick handler.
  • it allows us to preserve onclick as it is currently implemented.
  • it still allows us to simplify our templates and handler implementation.
  • node-needs-kb-click can be removed or left very simplistic as long as we know in the templates when onklick should be used, and when it shouldn't.
  • it is a baby step. changing onclick is disruptive and it is very hard to test how disruptive a change it is.

Changed 12 years ago by davidb

Attachment: 3461-kb-click.5.diff added

I think this is a decent baby step towards consistent kb based click.

comment:9 Changed 12 years ago by davidb

Bill, I'd like your nod before I commit this dijit core work. (If you can find the time)

comment:10 Changed 12 years ago by davidb

Resolution: fixed
Status: assignedclosed

(In [9299]) Fixes #3461: added new event type 'onklick'.

comment:11 Changed 12 years ago by davidb

(Approval from Bill via email)

comment:12 Changed 11 years ago by bill

(In [12159]) slight optimization from cougar; "w" var is not necessary since dojo.connect's third arguments make "this" valid. refs #3461. !strict

Note: See TracTickets for help on using tickets.