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.

Sign up for GitHub

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

Jump to bottom

Checkbox Example (Two State): Activate checkbox only on keyup #2518

Merged
merged 3 commits into from
Nov 10, 2022
Merged

Checkbox Example (Two State): Activate checkbox only on keyup #2518

merged 3 commits into from
Nov 10, 2022

Conversation

sivakusayan
Copy link
Contributor

Fixes #2425.

I chose to move this logic to a keyup handler to maintain consistency with HTML checkboxes. If this fix is okay, we should probably make a similar fix to the Mixed Checkbox example.

@mcking65
Copy link
Contributor

@howard-e @alflennik

Note the failure of workflow
Trigger PR update for WAI-APG site / create-pr-wai

@mcking65 mcking65 requested a review from jongund November 1, 2022 18:51
jongund
jongund approved these changes Nov 1, 2022
View reviewed changes
Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the changes and it works great.
I think "spacebar" in the documentation needs to change to "space" to pass the spelling rules

@sivakusayan
Copy link
Contributor Author

sivakusayan commented Nov 1, 2022 •
edited
Loading

Thank you for the review, I fixed the spell check 🙂
I also removed the event.preventDefault() call in the new keyup handler since I realized it's no longer needed. I don't think browsers do anything when the spacebar is released — I'm guessing it was there to prevent the scrolling issue when it was previously a keydown handler.

@mcking65 mcking65 changed the title Make sure checkbox only activates on keyup Checkbox Example (Two State): Activate checkbox only on keyup Nov 3, 2022
@mcking65 mcking65 mentioned this pull request Nov 3, 2022
Merged
alflennik
alflennik approved these changes Nov 9, 2022
View reviewed changes
Copy link
Contributor

@alflennik alflennik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, and it is working in my quick tests. @mcking65 after you merge this to main I'll copy these changes the move-examples branch which reorganizes all the source code.

@mcking65 mcking65 merged commit d7db313 into w3c:main Nov 10, 2022
@sivakusayan sivakusayan mentioned this pull request Nov 15, 2022
Closed
@sivakusayan sivakusayan mentioned this pull request Jun 8, 2023
Merged
mcking65 pushed a commit that referenced this pull request Jun 26, 2023
@sivakusayan
Checkbox Example (Mixed-State): Toggle on keyup to prevent continuous…
049893e 
… toggling (pull #2722)

Modified JavaScript for Checkbox Example (Mixed-State) so that the state of the checkbox does not change until key up. This change prevents continuous toggling if the user holds down the space key.

This is the same change that was made for Checkbox Example (Two State) in pull request #2518.

This commit fully resolves issue #2425.
@howard-e howard-e mentioned this pull request Jul 31, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alflennik alflennik alflennik approved these changes

@jongund jongund jongund approved these changes

@smhigley smhigley Awaiting requested review from smhigley

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Checkbox examples should not repeatedly toggle while holding down spacebar
4 participants
@sivakusayan @mcking65 @jongund @alflennik