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

Editable Combobox With List Autocomplete: Remove "onBlur" events. #1699

Merged
merged 6 commits into from
Feb 21, 2021
+106 −9
Merged

Editable Combobox With List Autocomplete: Remove "onBlur" events. #1699

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a32a1a6
issue 1619: Remove 'blur' event on combobox
Dec 21, 2020
733c67f
issue 1619: add tests
Dec 22, 2020
acc7577
Satisfy linter
Jan 12, 2021
f7c6f5c
added the button to the list of mousedown elements to be tested
jongund Jan 27, 2021
c44d05c
fixed spelling issues with test file
jongund Jan 27, 2021
c169ff5
fixed spelling issues with test file
jongund Jan 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions examples/combobox/js/combobox-autocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ class ComboboxAutocomplete {
'focus',
this.onComboboxFocus.bind(this)
);
this.comboboxNode.addEventListener('blur', this.onComboboxBlur.bind(this));

document.body.addEventListener(
'mouseup',
Copy link

@oliviertassinari oliviertassinari Feb 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

One note, mouseup fire when dragging the body's scrollbar, blur doesn't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there another issue to resolve here? Does that mean that scrolling the page unexpectedly closes the dropdown? As a screen reader user, I can't test this, so I'm not quite clear on what the problem is.

Copy link

@oliviertassinari oliviertassinari Feb 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I was raising a potential regression (not sure about the importance of it) on the UX for mouse users. I'm not aware of any impact on screen readers.

I can't reproduce the issue on https://raw.githack.com/w3c/aria-practices/combobox-VOMac-fix/examples/combobox/combobox-autocomplete-list.html.

this.onBackgroundMouseUp.bind(this),
true
);

// initialize pop up menu

Expand Down Expand Up @@ -507,11 +512,17 @@ class ComboboxAutocomplete {
this.setCurrentOptionStyle(null);
}

onComboboxBlur() {
this.comboboxHasVisualFocus = false;
this.setCurrentOptionStyle(null);
this.removeVisualFocusAll();
setTimeout(this.close.bind(this, false), 300);
onBackgroundMouseUp(event) {
if (
!this.comboboxNode.contains(event.target) &&
!this.listboxNode.contains(event.target) &&
!this.buttonNode.contains(event.target)
) {
this.comboboxHasVisualFocus = false;
this.setCurrentOptionStyle(null);
this.removeVisualFocusAll();
setTimeout(this.close.bind(this, true), 300);
}
}

onButtonClick() {
Expand Down Expand Up @@ -563,7 +574,6 @@ window.addEventListener('load', function () {
var comboboxNode = combobox.querySelector('input');
var buttonNode = combobox.querySelector('button');
var listboxNode = combobox.querySelector('[role="listbox"]');
var cba = new ComboboxAutocomplete(comboboxNode, buttonNode, listboxNode);
cba.init();
new ComboboxAutocomplete(comboboxNode, buttonNode, listboxNode);
}
});
87 changes: 87 additions & 0 deletions test/tests/combobox_autocomplete-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const ex = {
optionsSelector: '#ex1 [role="option"]',
buttonSelector: '#ex1 button',
numAOptions: 5,
exampleHeadingSelector: '.example-header',
};

const waitForFocusChange = async (t, textboxSelector, originalFocus) => {
Expand Down Expand Up @@ -850,6 +851,92 @@ ariaTest(
}
);

ariaTest(
'Clicking outside of textbox and listbox when focus is on listbox closes listbox',
exampleFile,
'test-additional-behavior',
async (t) => {
// Send key "a" to open listbox then key "ARROW_DOWN" to put the focus on the listbox
await t.context.session
.findElement(By.css(ex.textboxSelector))
.sendKeys('a', Key.ARROW_DOWN);

// click outside the listbox
await t.context.session
.findElement(By.css(ex.exampleHeadingSelector))
.click();

await t.context.session.wait(
async function () {
let listbox = await t.context.session.findElement(
By.css(ex.listboxSelector)
);
return !(await listbox.isDisplayed());
},
t.context.waitTime,
'Error waiting for listbox to close after outside click'
);

// Confirm the listbox is closed and the textboxed is cleared
await assertAttributeValues(
t,
ex.textboxSelector,
'aria-expanded',
'false'
);
t.is(
await t.context.session
.findElement(By.css(ex.textboxSelector))
.getAttribute('value'),
'a',
'Click outside of a textbox will close the testbox without selecting the highlighted value'
);
}
);

ariaTest(
'Clicking outside of textbox and listbox when focus is on textbox closes listbox',
exampleFile,
'test-additional-behavior',
async (t) => {
// Send key "a" to open listbox to put focus on textbox
await t.context.session
.findElement(By.css(ex.textboxSelector))
.sendKeys('a');

// click outside the listbox
await t.context.session
.findElement(By.css(ex.exampleHeadingSelector))
.click();

await t.context.session.wait(
async function () {
let listbox = await t.context.session.findElement(
By.css(ex.listboxSelector)
);
return !(await listbox.isDisplayed());
},
t.context.waitTime,
'Error waiting for listbox to close after outside click'
);

// Confirm the listbox is closed and the textboxed is cleared
await assertAttributeValues(
t,
ex.textboxSelector,
'aria-expanded',
'false'
);
t.is(
await t.context.session
.findElement(By.css(ex.textboxSelector))
.getAttribute('value'),
'a',
'Click outside of a textbox will close the testbox without selecting the highlighted value'
);
}
);

ariaTest(
'left arrow from focus on list puts focus on listbox and moves cursor right',
exampleFile,
Expand Down