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

ARIA 1.1 Combobox with Listbox Popup Examples - issue with onBlur #982

Closed
gaurav5430 opened this issue Feb 5, 2019 · 9 comments · Fixed by #1276
Closed

ARIA 1.1 Combobox with Listbox Popup Examples - issue with onBlur #982

gaurav5430 opened this issue Feb 5, 2019 · 9 comments · Fixed by #1276
Assignees
Labels
bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern
Milestone

Comments

@gaurav5430
Copy link

As i start typing 'A', the listbox appears with options Apple, Artichoke, Asparagus.
I press down, it selects Apple,
the listbox is still open,
now if i click on Artichoke
the listbox closes, but the selection is Apple.
I would expect the selection to be Artichoke, since i clicked on it.

To me, the issue seems to be because of the blur event.
the blur handler is called before the click handler in this case, and it selects the already selected item (which was selected by keyboard) and then the click handler is not called at all.

Though in the usual case, where we don't use keyboard selection, the blur and click events are fired one after another.

so possibly something to do with some kind of delay between blur and click ?
it seems that if blur event hides the clickable element , the click event does not go through.

@gaurav5430
Copy link
Author

It seems to work fine when there is no previously selected element.
This might be due to the fact that checkSelection returns early in this case and the click event gets fired before the dom element is actually hidden from view.

in the case where there is already a selection, checkSelection takes more time and the click event does not get fired as meanwhile the dom element has already been hidden/removed from view.

@mcking65 mcking65 added bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern labels Feb 5, 2019
@mcking65 mcking65 added this to the 1.1 APG Release 4 milestone Feb 5, 2019
@mcking65
Copy link
Contributor

mcking65 commented Feb 5, 2019

@gaurav5430 commented:

As i start typing 'A', the listbox appears with options Apple, Artichoke, Asparagus.
I press down, it selects Apple,
the listbox is still open,
now if i click on Artichoke
the listbox closes, but the selection is Apple.
I would expect the selection to be Artichoke, since i clicked on it.

I would expect the same, good catch!

@gaurav5430
Copy link
Author

The way I handle it in my combobox is to prevent the blur from being fired if the click is from within the listbox, and manually fire the blur at the end of the click handler.
Is it ok if I create a PR for this fix?

@Collaborator12
Copy link

Thanks for giving at least some solution to this issue 👍

@Collaborator12
Copy link

The way I handle it in my combobox is to prevent the blur from being fired if the click is from within the listbox, and manually fire the blur at the end of the click handler.
Is it ok if I create a PR for this fix?

@gaurav5430 do you know why the blur event is needed ?

@gaurav5430
Copy link
Author

@Abhishek-Kanitkar The blur event is used in this case to hide the popup, when the user moves focus away from the input using a keyboard.
Similar to how we have something like onClickOutside for checking an outside click which would mean focus has moved away from the input and we should hide the popup, this is for similar functionality when using a keyboard to move focus away from the input.

@mcking65 mcking65 modified the milestones: 1.1 APG Release 4, 1.2 CR Aug 13, 2019
@smhigley smhigley self-assigned this Nov 19, 2019
@jongund
Copy link
Contributor

jongund commented Jan 21, 2020

This issue should be fixed in pull request 1276

@carmacleod
Copy link
Contributor

I can confirm that this issue will be fixed after PR #1276 is merged.

I tested with the following preview links, and the issue does not occur there:

@mcking65
Copy link
Contributor

mcking65 commented Feb 7, 2020

Excellent, @carmacleod, thank you for the testing!

I'll use the cool new github issue/pr linking feature to link this issue to #1276.

mcking65 added a commit that referenced this issue Feb 14, 2020
…raction, visual design, and more (pull #1276)

Fixes #785, #982, #983, #988, #1261, #1265, and #1268 with the following changes:
* updated JavaScript to use single prototype
* updated escape key behavior
* removed unused files
* fixed regression issues for escape key
* updated tests for single and double escape key tests
* fixed focus bug on enter and removed use of keycode property
* fixed bug in opening list and improved property names for visual focus flags
* fixed bug in opening list with alt key pressed
* fixed bug with enter key not updating aria-expanded
* fixed bugs with down arrow
* added documentation and tests for ALT Down Arrow
* fixed some styling bugs
* Use only one SVG image to show listbox state
* updated CSS for styling listbox focus
* fixed scrolling issue in listbox
* adjusted position of svg image button
* fixed onclik bug where not selecting options when autocomplete is list or none
* updated test for autocomplete, list and none
* fixed bug with onclick behavior and documented option filtering as users type
* updated documentation about filtering of options
* Typo: character -> characters
* fixed option filter bug with autocomplete=none
* improved description of when listbox opens
* use lowercase "the" in sentence
* Minor editorial revision to alt+down in textbox keyboard table
* udpated test file to check for aria-selected on option when listbox opens
* fixed aria-selected tests
* add tests for aria-selected to key down tests
* Add aria-hidden=false and focusable=false to svg in button
* Use fewer descendant combinators in selectors
* Call function instead of setting a property on elements
* Don't call setValue() when hitting backspace, to avoid moving the cursor to the end
* Remove superfluous isPrintableCharacter() call
* Remove unused variable textContent
* Declare option outside for loop. Also use null instead of false
* Set scrollTop once
* Declare index variable in the if block
* Move isPrintableCharacter to ComboboxAutocomplete.prototype
* Add information about the id attribute for all combobox examples; fixes #785

Co-authored-by: Carolyn MacLeod <Carolyn_MacLeod@ca.ibm.com>
Co-authored-by: Matt King <a11yThinker@Gmail.com>
Co-authored-by: Valerie Young <spectranaut@gmail.com>
Co-authored-by: Simon Pieters <zcorpan@gmail.com>
michael-n-cooper pushed a commit that referenced this issue Feb 14, 2020
Combobox Examples with listbox popup: Fix Escape behavior, mouse interaction, visual design, and more  (pull #1276)

Fixes #785, #982, #983, #988, #1261, #1265, and #1268 with the following changes:
* updated JavaScript to use single prototype
* updated escape key behavior
* removed unused files
* fixed regression issues for escape key
* updated tests for single and double escape key tests
* fixed focus bug on enter and removed use of keycode property
* fixed bug in opening list and improved property names for visual focus flags
* fixed bug in opening list with alt key pressed
* fixed bug with enter key not updating aria-expanded
* fixed bugs with down arrow
* added documentation and tests for ALT Down Arrow
* fixed some styling bugs
* Use only one SVG image to show listbox state
* updated CSS for styling listbox focus
* fixed scrolling issue in listbox
* adjusted position of svg image button
* fixed onclik bug where not selecting options when autocomplete is list or none
* updated test for autocomplete, list and none
* fixed bug with onclick behavior and documented option filtering as users type
* updated documentation about filtering of options
* Typo: character -> characters
* fixed option filter bug with autocomplete=none
* improved description of when listbox opens
* use lowercase "the" in sentence
* Minor editorial revision to alt+down in textbox keyboard table
* udpated test file to check for aria-selected on option when listbox opens
* fixed aria-selected tests
* add tests for aria-selected to key down tests
* Add aria-hidden=false and focusable=false to svg in button
* Use fewer descendant combinators in selectors
* Call function instead of setting a property on elements
* Don't call setValue() when hitting backspace, to avoid moving the cursor to the end
* Remove superfluous isPrintableCharacter() call
* Remove unused variable textContent
* Declare option outside for loop. Also use null instead of false
* Set scrollTop once
* Declare index variable in the if block
* Move isPrintableCharacter to ComboboxAutocomplete.prototype
* Add information about the id attribute for all combobox examples; fixes #785

Co-authored-by: Carolyn MacLeod <Carolyn_MacLeod@ca.ibm.com>
Co-authored-by: Matt King <a11yThinker@Gmail.com>
Co-authored-by: Valerie Young <spectranaut@gmail.com>
Co-authored-by: Simon Pieters <zcorpan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code defects; not for inaccurate prose Example Page Related to a page containing an example implementation of a pattern
Projects
None yet
6 participants