Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Button: Space should only activate on keyup #610

Closed
sh0ji opened this issue Feb 21, 2018 · 6 comments

Comments

@sh0ji
Copy link
Contributor

@sh0ji sh0ji commented Feb 21, 2018

The synthetic click event that happens on native <button> elements occurs on keydown for Enter and keyup for Space, but the button pattern doesn't describe this behavior and the example does not exhibit it. For instance, holding down the space bar on the "Mute" button toggles it off and on until you release the space bar--it should only trigger once on key release.

I tested this behavior using this codepen example on Firefox, Chrome, and Safari on Mac to be certain about it. Holding down space bar on the "real button" only triggers the click even on release, whereas holding down enter triggers it repeatedly until release. I added a "fake button" to demonstrate how this behavior can be replicated in JavaScript.

@kleinfreund

This comment has been minimized.

Copy link
Contributor

@kleinfreund kleinfreund commented Apr 20, 2018

I will address this issue in a follow-up pull request to #609, once it is merged. Have it implemented locally.

@mcking65

This comment has been minimized.

Copy link
Contributor

@mcking65 mcking65 commented Aug 26, 2018

@kleinfreund, are you still planning to submit a PR for this?

@spectranaut, does this impact button tests in PR #849?

@kleinfreund

This comment has been minimized.

Copy link
Contributor

@kleinfreund kleinfreund commented Aug 27, 2018

@mcking65 I didn’t have this in mind anymore. I simply forgot about it. Sorry for that; I’m on it now.

One thing: For the toggle button’s click event, it is explicitly checked whether the target element is something like a button (i.e. either is a <button> tag or has the button role).

if (event.target.getAttribute('role') === 'button' || event.target.tagName === 'button') {
  toggleButtonState(event);
}

This check is not in place for any other event and only for the toggle button. In particular, the action button (implemented as a <div> element with role button) just activates the button without this check.

I would like to remove this check. Currently, it’s not helpful because the examples will fulfill either criteria anyway. Otherwise, we would need to introduce such safety checks in a lot of other places, too.

Regarding the tests in #849: As far as I can tell from the test file, the tests don’t distinquish between event types. Instead, a fixed amount of time is waited after which the test condition is evaluated.

@spectranaut

This comment has been minimized.

Copy link
Contributor

@spectranaut spectranaut commented Aug 27, 2018

hi @mcking65, nope, this bug does not effect the regression tests. The tests send a key and wait for a state change -- they do not test whether the state change happens at the time of keypress or keyup.

mcking65 added a commit that referenced this issue Sep 13, 2018
To resolve issue #610, activate  buttons on keyup when space is pressed.
Enter key behavior is not changed.
Makes behavior consistent with HTML button elements.
michael-n-cooper added a commit that referenced this issue Sep 13, 2018
Button examples: Activate buttons on keyup for space key (pull #858)

To resolve issue #610, activate  buttons on keyup when space is pressed.
Enter key behavior is not changed.
Makes behavior consistent with HTML button elements.
spectranaut added a commit to bocoup/aria-practices that referenced this issue Sep 13, 2018
To resolve issue w3c#610, activate  buttons on keyup when space is pressed.
Enter key behavior is not changed.
Makes behavior consistent with HTML button elements.
spectranaut added a commit to bocoup/aria-practices that referenced this issue Sep 14, 2018
To resolve issue w3c#610, activate  buttons on keyup when space is pressed.
Enter key behavior is not changed.
Makes behavior consistent with HTML button elements.
@kleinfreund

This comment has been minimized.

Copy link
Contributor

@kleinfreund kleinfreund commented Oct 16, 2018

This can be closed, no?

@sh0ji

This comment has been minimized.

Copy link
Contributor Author

@sh0ji sh0ji commented Oct 16, 2018

Yep, this was resolved by #858. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.