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

selectlist: Implement new keyboard behavior #42068

Merged
merged 1 commit into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -130,68 +130,55 @@

promise_test(async () => {
const selectList = document.getElementById("selectList2");
let input_event_count = 0;
let change_event_count = 0;
const events = [];

selectList.addEventListener("input", (e) => {
assert_true(e.composed, "input event should be composed");
assert_equals(input_event_count, 0, "input event should not fire twice");
assert_equals(change_event_count, 0, "input event should not fire before change");
input_event_count++;
assert_true(e.composed, "input event should be composed.");
events.push('input');
});
selectList.addEventListener("change", (e) => {
assert_false(e.composed, "change event should not be composed");
assert_equals(input_event_count, 1, "change event should fire after input");
assert_equals(change_event_count, 0, "change event should not fire twice");
change_event_count++;
assert_false(e.composed, "change event should not be composed.");
events.push('change');
});

await clickOn(selectList);
assert_true(selectList.open);
await test_driver.send_keys(selectList, KEY_CODE_MAP.Enter);
assert_false(selectList.open);
assert_equals(selectList.value, "one");
assert_equals(input_event_count, 0, "input event shouldn't fire if value wasn't changed");
assert_equals(change_event_count, 0, "change event shouldn't fire if value wasn't changed");
assert_array_equals(events, [], "input and change shouldn't fire if value wansn't changed.");

await clickOn(selectList);
assert_true(selectList.open);
await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList.value, "two", "value should change when user switches options with arrow key");
assert_equals(input_event_count, 1, "input event should fire when user switches options with arrow key");
assert_equals(change_event_count, 0, "change event shouldn't fire until popover is closed");
assert_equals(selectList.value, "one", "value shouldn't change when user switches options with arrow key.");
assert_array_equals(events, ['input'], "input event should fire when user switches options with arrow key.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.Enter);
assert_equals(selectList.value, "two");
assert_equals(input_event_count, 1, "input event should have fired");
assert_equals(change_event_count, 1, "change event should have fired");
}, "<selectlist> should fire input and change events when new option is selected");
assert_array_equals(events, ['input', 'input', 'change'], "input and change should fire after pressing enter.");
}, "<selectlist> should fire input and change events when new option is selected.");

promise_test(async () => {
const selectList = document.getElementById("selectList3");
let input_event_count = 0;
let change_event_count = 0;
const events = [];

selectList.addEventListener("input", (e) => {
assert_true(e.composed, "input event should be composed");
assert_equals(input_event_count, 0, "input event should not fire twice");
assert_equals(change_event_count, 0, "input event should not fire before change");
input_event_count++;
assert_true(e.composed, "input event should be composed.");
events.push('input');
});
selectList.addEventListener("change", (e) => {
assert_false(e.composed, "change event should not be composed");
assert_equals(input_event_count, 1, "change event should fire after input");
assert_equals(change_event_count, 0, "change event should not fire twice");
change_event_count++;
assert_false(e.composed, "change event should not be composed.");
events.push('change');
});

await clickOn(selectList);
assert_true(selectList.open);
await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_array_equals(events, ['input'], "input event should have fired after ArrowDown.");
await test_driver.send_keys(selectList, KEY_CODE_MAP.Enter);
assert_equals(input_event_count, 1, "input event should have fired");
assert_equals(change_event_count, 1, "change event should have fired");
}, "<selectlist> should fire input and change events even when new selected option has the same value as the old");
assert_array_equals(events, ['input', 'input', 'change'], "input and change should fire after pressing Enter.");
}, "<selectlist> should fire input and change events even when new selected option has the same value as the old.");

promise_test(async () => {
const selectList = document.getElementById("selectList4");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
<!DOCTYPE html>
<link rel=author href="mailto:jarhar@chromium.org">
<link rel=help href="https://bugs.chromium.org/p/chromium/issues/detail?id=1422275">
<link rel=help href="https://github.com/openui/open-ui/issues/433#issuecomment-1452461404">
<link rel=help href="https://github.com/openui/open-ui/issues/386#issuecomment-1452469497">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testdriver-actions.js"></script>

<form></form>

<div id=notform>
<selectlist id=defaultbutton>
<option class=one>one</option>
<option class=two>two</option>
<option class=three>three</option>
</selectlist>

<selectlist id=custombutton>
<button type=selectlist>custom button</button>
<option class=one>one</option>
<option class=two>two</option>
<option class=three>three</option>
</selectlist>
</div>

<script>
const Enter = '\uE007';
const Escape = '\uE00C';
const ArrowLeft = '\uE012';
const ArrowUp = '\uE013';
const ArrowRight = '\uE014';
const ArrowDown = '\uE015';
const Space = ' ';
const form = document.querySelector('form');
const notform = document.getElementById('notform');

for (const id of ['defaultbutton', 'custombutton']) {
const selectlist = document.getElementById(id);

async function closeListbox() {
await test_driver.click(selectlist);
}

function addCloseCleanup(t) {
t.add_cleanup(async () => {
if (selectlist.matches(':open')) {
await closeListbox();
}
if (selectlist.matches(':open')) {
throw new Error('selectlist failed to close!');
}
selectlist.value = 'one';
});
}

promise_test(async t => {
addCloseCleanup(t);
// TODO(http://crbug.com/1350299): When focus for custom buttons is fixed,
// then we shouldn't need to explicitly focus the custom button like this
// anymore.
const customButton = selectlist.querySelector('button');
if (customButton) {
customButton.focus();
} else {
selectlist.focus();
}
assert_false(selectlist.matches(':open'),
'The selectlist should initially be closed.');
await test_driver.send_keys(selectlist, Space);
assert_true(selectlist.matches(':open'),
'The selectlist should be open after pressing space.');
}, `${id}: When the listbox is closed, spacebar should open the listbox.`);

promise_test(async t => {
addCloseCleanup(t);
selectlist.value = 'two';
selectlist.focus();
assert_false(selectlist.matches(':open'),
'The selectlist should initially be closed.');

await test_driver.send_keys(selectlist, ArrowLeft);
assert_true(selectlist.matches(':open'),
'Arrow left should open the listbox.');
assert_equals(selectlist.value, 'two',
'Arrow left should not change the selected value.');
await closeListbox();

await test_driver.send_keys(selectlist, ArrowUp);
assert_true(selectlist.matches(':open'),
'Arrow up should open the listbox.');
assert_equals(selectlist.value, 'two',
'Arrow up should not change the selected value.');
await closeListbox();

await test_driver.send_keys(selectlist, ArrowRight);
assert_true(selectlist.matches(':open'),
'Arrow right should open the listbox.');
assert_equals(selectlist.value, 'two',
'Arrow right should not change the selected value.');
await closeListbox();

await test_driver.send_keys(selectlist, ArrowDown);
assert_true(selectlist.matches(':open'),
'Arrow down should open the listbox.');
assert_equals(selectlist.value, 'two',
'Arrow down should not change the selected value.');
}, `${id}: When the listbox is closed, all arrow keys should open the listbox.`);

promise_test(async t => {
addCloseCleanup(t);

// TODO(http://crbug.com/1350299): When focus for custom buttons is fixed,
// then we shouldn't need to explicitly use the custom button like this
// anymore.
const customButton = selectlist.querySelector('button');
if (customButton) {
await test_driver.send_keys(customButton, Enter);
} else {
await test_driver.send_keys(selectlist, Enter);
}
assert_false(selectlist.matches(':open'),
'Enter should not open the listbox when outside a form.');

form.appendChild(selectlist);
let formWasSubmitted = false;
form.addEventListener('submit', event => {
event.preventDefault();
formWasSubmitted = true;
}, {once: true});
if (customButton) {
await test_driver.send_keys(customButton, Enter);
} else {
await test_driver.send_keys(selectlist, Enter);
}
assert_true(formWasSubmitted,
'Enter should submit the form when the listbox is closed.');
assert_false(selectlist.matches(':open'),
'Enter should not open the listbox when it is in a form.');
}, `${id}: When the listbox is closed, the enter key should submit the form or do nothing.`);

promise_test(async t => {
addCloseCleanup(t);
const optionOne = selectlist.querySelector('.one');
const optionTwo = selectlist.querySelector('.two');
const optionThree = selectlist.querySelector('.three');

selectlist.value = 'two';
await test_driver.click(selectlist);
assert_true(selectlist.matches(':open'),
'The selectlist should open when clicked.');
assert_equals(document.activeElement, optionTwo,
'The selected option should receive initial focus.');

await test_driver.send_keys(document.activeElement, ArrowDown);
assert_equals(document.activeElement, optionThree,
'The next option should receive focus when the down arrow key is pressed.');
assert_equals(selectlist.value, 'two',
'The selectlists value should not change when focusing another option.');

await test_driver.send_keys(document.activeElement, ArrowUp);
assert_equals(document.activeElement, optionTwo,
'The previous option should receive focus when the up arrow key is pressed.');
assert_equals(selectlist.value, 'two',
'The selectlists value should not change when focusing another option.');

await test_driver.send_keys(document.activeElement, ArrowUp);
assert_equals(document.activeElement, optionOne,
'The first option should be selected.');
assert_equals(selectlist.value, 'two',
'The selectlists value should not change when focusing another option.');

await test_driver.send_keys(document.activeElement, Enter);
assert_false(selectlist.matches(':open'),
'The listbox should be closed after pressing enter.');
assert_equals(selectlist.value, 'one',
'The selectlists value should change after pressing enter on a different option.');
}, `${id}: When the listbox is open, the enter key should commit the selected option.`);
}
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,28 @@
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<selectlist id="selectList0">
<div id="selectList0-button0" slot="button" behavior="button" tabindex="0">button</div>
<option>one</option>
<option>two</option>
<option>three</option>
<button id="selectList0-button0" type=selectlist>button</button>
<option class=one>one</option>
<option class=two>two</option>
<option class=three>three</option>
</selectlist>

<selectlist id="selectList1">
<option id="selectList1-child0">one</option>
</selectlist>

<selectlist id="selectList2" disabled>
<div id="selectList2-button0" slot="button" behavior="button" tabindex="0">button</div>
<button id="selectList2-button0" type=selectlist>button</button>
<option disabled>one</option>
<option>two</option>
<option>three</option>
</selectlist>

<selectlist id="selectList3">
<div id="selectList3-button0" slot="button" behavior="button" tabindex="0">button</div>
<option>one</option>
<button id="selectList3-button0" type=selectlist>button</button>
<option class=one>one</option>
<option disabled>two</option>
<option>three</option>
<option class=three>three</option>
</selectlist>
<script>
// See https://w3c.github.io/webdriver/#keyboard-actions
Expand All @@ -52,31 +52,45 @@
assert_false(selectList.open, "selectlist should not be initially open");

await test_driver.send_keys(button, KEY_CODE_MAP.Enter);
assert_true(selectList.open, "Enter key should open selectlist");
assert_false(selectList.open, "Enter key shouldn't open selectlist");
await test_driver.send_keys(button, KEY_CODE_MAP.Space);
assert_true(selectList.open, "Space key should open selectlist");
assert_equals(selectList.value, "one");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList.value, "two", "Down arrow should go to next option");
assert_equals(document.activeElement, selectList.querySelector('.two'),
"Down arrow should focus the next option.");
assert_equals(selectList.value, "one", "Down arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList.value, "three", "Down arrow should go to next option");
assert_equals(document.activeElement, selectList.querySelector('.three'),
"Down arrow should focus the next option.");
assert_equals(selectList.value, "one", "Down arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList.value, "three", "Down arrow should do nothing if already at the last option");
assert_equals(document.activeElement, selectList.querySelector('.three'),
"Down arrow should do nothing if already at the last option.");
assert_equals(selectList.value, "one", "Down arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowUp);
assert_equals(selectList.value, "two", "Up arrow should go to the previous option");
assert_equals(document.activeElement, selectList.querySelector('.two'),
"Up arrow should focus the previous option.");
assert_equals(selectList.value, "one", "Up arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowUp);
assert_equals(selectList.value, "one", "Up arrow should go to the previous option");
assert_equals(document.activeElement, selectList.querySelector('.one'),
"Up arrow should focus the previous option.");
assert_equals(selectList.value, "one", "Up arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.ArrowUp);
assert_equals(selectList.value, "one", "Up arrow should do nothing if already at the first option");
assert_equals(document.activeElement, selectList.querySelector('.one'),
"Up arrow should do nothing if already at the first option.");
assert_equals(selectList.value, "one", "Up arrow should not commit the newly focused option.");

await test_driver.send_keys(selectList, KEY_CODE_MAP.Enter);
assert_false(selectList.open, "Enter key should close selectlist");

await test_driver.send_keys(selectList, " ");
await test_driver.send_keys(button, KEY_CODE_MAP.Space);
assert_true(selectList.open, "Space key should open selectlist");

// This behavior is suspicious (since Space key can open the selectlist),
Expand Down Expand Up @@ -111,13 +125,18 @@
assert_false(selectList3.open, "selectlist should not be initially open");

await test_driver.send_keys(selectList3Button, KEY_CODE_MAP.Enter);
assert_true(selectList3.open, "Enter key should open selectlist");
assert_false(selectList3.open, "Enter key shouldn't open selectlist");

await test_driver.send_keys(selectList3Button, KEY_CODE_MAP.Space);
assert_true(selectList3.open, "Space key should open selectlist");
assert_equals(selectList3.value, "one");

await test_driver.send_keys(selectList3, KEY_CODE_MAP.ArrowDown);
assert_equals(selectList3.value, "three", "Down arrow should go to next non-disabled option");
assert_equals(document.activeElement, selectList3.querySelector('.three'),
"Down arrow should go to next non-disabled option");

await test_driver.send_keys(selectList3, KEY_CODE_MAP.ArrowUp);
assert_equals(selectList3.value, "one", "Up arrow should go to the previous non-disabled option");
assert_equals(document.activeElement, selectList3.querySelector('.one'),
"Up arrow should go to the previous non-disabled option");
}, "Validate Enter, Up/Down Arrow keyboard accessibility support for disabled <selectlist>");
</script>