From fc59f2787747d9247aa8a3093ab8054e204e1b0c Mon Sep 17 00:00:00 2001 From: Andrew Leach Date: Fri, 23 Feb 2024 15:12:41 +0000 Subject: [PATCH 1/5] Added new tests --- src/index.ts | 6 ++---- test/test.js | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/index.ts b/src/index.ts index 5164a8c..190fde2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -97,7 +97,7 @@ export default class Combobox { Array.from(this.list.querySelectorAll('[role="option"]:not([aria-disabled="true"])')) .filter(visible)[0] ?.setAttribute('data-combobox-option-default', 'true') - } else if (this.firstOptionSelectionMode === 'selected') { + } else if (this.firstOptionSelectionMode === 'selected' && visible(this.list)) { this.navigate(1) } } @@ -142,9 +142,7 @@ export default class Combobox { el.removeAttribute('aria-selected') } - if (this.firstOptionSelectionMode === 'active') { - this.indicateDefaultOption() - } + this.indicateDefaultOption() } } diff --git a/test/test.js b/test/test.js index 3368cce..46c8f47 100644 --- a/test/test.js +++ b/test/test.js @@ -285,6 +285,15 @@ describe('combobox-nav', function () { assert.equal(document.querySelector('[data-combobox-option-default]'), options[0]) }) + it('first item remains active when typing', () => { + const text = 'R2-D2' + for (let i = 0; i < text.length; i++) { + press(input, text[i]) + } + + assert.equal(document.querySelector('[data-combobox-option-default]'), options[0]) + }) + it('applies default option on Enter', () => { let commits = 0 document.addEventListener('combobox-commit', () => commits++) @@ -349,6 +358,15 @@ describe('combobox-nav', function () { assert.equal(list.children[0].getAttribute('aria-selected'), 'true') }) + it('first item remains selected when typing', () => { + const text = 'R2-D2' + for (let i = 0; i < text.length; i++) { + press(input, text[i]) + } + + assert.equal(list.children[0].getAttribute('aria-selected'), 'true') + }) + it('indicates first option when restarted', () => { combobox.stop() combobox.start() From dfbf7c5c3e56a26568c366702ebffa21d1d27e03 Mon Sep 17 00:00:00 2001 From: Andrew Leach Date: Fri, 23 Feb 2024 17:37:25 +0000 Subject: [PATCH 2/5] Spike on separation of concerns --- src/index.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 190fde2..5a33655 100644 --- a/src/index.ts +++ b/src/index.ts @@ -80,6 +80,7 @@ export default class Combobox { ;(this.input as HTMLElement).addEventListener('keydown', this.keyboardEventHandler) this.list.addEventListener('click', commitWithElement) this.indicateDefaultOption() + this.selectFirstItemIfNeeded() } stop(): void { @@ -97,7 +98,11 @@ export default class Combobox { Array.from(this.list.querySelectorAll('[role="option"]:not([aria-disabled="true"])')) .filter(visible)[0] ?.setAttribute('data-combobox-option-default', 'true') - } else if (this.firstOptionSelectionMode === 'selected' && visible(this.list)) { + } + } + + selectFirstItemIfNeeded(): void { + if (this.firstOptionSelectionMode === 'selected') { this.navigate(1) } } @@ -142,7 +147,9 @@ export default class Combobox { el.removeAttribute('aria-selected') } - this.indicateDefaultOption() + if (this.firstOptionSelectionMode === 'active') { + this.indicateDefaultOption() + } } } @@ -188,6 +195,7 @@ function keyboardBindings(event: KeyboardEvent, combobox: Combobox) { default: if (event.ctrlKey) break combobox.clearSelection() + combobox.selectFirstItemIfNeeded() } } From 4365fb9948d201336bfba91ce4ee31db159c5ad5 Mon Sep 17 00:00:00 2001 From: Andrew Leach Date: Fri, 23 Feb 2024 18:21:08 +0000 Subject: [PATCH 3/5] More changes --- src/index.ts | 19 +++++++------------ test/test.js | 10 ++++++++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/index.ts b/src/index.ts index 5a33655..df48fc6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -80,7 +80,6 @@ export default class Combobox { ;(this.input as HTMLElement).addEventListener('keydown', this.keyboardEventHandler) this.list.addEventListener('click', commitWithElement) this.indicateDefaultOption() - this.selectFirstItemIfNeeded() } stop(): void { @@ -98,11 +97,7 @@ export default class Combobox { Array.from(this.list.querySelectorAll('[role="option"]:not([aria-disabled="true"])')) .filter(visible)[0] ?.setAttribute('data-combobox-option-default', 'true') - } - } - - selectFirstItemIfNeeded(): void { - if (this.firstOptionSelectionMode === 'selected') { + } else if (this.firstOptionSelectionMode === 'selected') { this.navigate(1) } } @@ -113,7 +108,7 @@ export default class Combobox { const focusIndex = els.indexOf(focusEl) if ((focusIndex === els.length - 1 && indexDiff === 1) || (focusIndex === 0 && indexDiff === -1)) { - this.clearSelection() + this.resetSelection() this.input.focus() return } @@ -146,10 +141,11 @@ export default class Combobox { for (const el of this.list.querySelectorAll('[aria-selected="true"]')) { el.removeAttribute('aria-selected') } + } - if (this.firstOptionSelectionMode === 'active') { - this.indicateDefaultOption() - } + resetSelection(): void { + this.clearSelection() + this.indicateDefaultOption() } } @@ -194,8 +190,7 @@ function keyboardBindings(event: KeyboardEvent, combobox: Combobox) { break default: if (event.ctrlKey) break - combobox.clearSelection() - combobox.selectFirstItemIfNeeded() + combobox.resetSelection() } } diff --git a/test/test.js b/test/test.js index 46c8f47..0368285 100644 --- a/test/test.js +++ b/test/test.js @@ -308,12 +308,18 @@ describe('combobox-nav', function () { assert.equal(document.querySelectorAll('[data-combobox-option-default]').length, 0) }) - it('resets default indication when selection cleared', () => { + it('resets default indication when selection reset', () => { combobox.navigate(1) - combobox.clearSelection() + combobox.resetSelection() assert.equal(document.querySelectorAll('[data-combobox-option-default]').length, 1) }) + it('removes default indication when selection cleared', () => { + combobox.navigate(1) + combobox.clearSelection() + assert.equal(document.querySelectorAll('[data-combobox-option-default]').length, 0) + }) + it('does not error when no options are visible', () => { assert.doesNotThrow(() => { document.getElementById('list-id').style.display = 'none' From 7182ef587de85db41fc1213579ff63f8d4eab6a2 Mon Sep 17 00:00:00 2001 From: Andrew Leach Date: Mon, 26 Feb 2024 11:14:44 +0000 Subject: [PATCH 4/5] PR feedback --- src/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index df48fc6..9c85298 100644 --- a/src/index.ts +++ b/src/index.ts @@ -79,7 +79,7 @@ export default class Combobox { this.input.addEventListener('input', this.inputHandler) ;(this.input as HTMLElement).addEventListener('keydown', this.keyboardEventHandler) this.list.addEventListener('click', commitWithElement) - this.indicateDefaultOption() + this.resetSelection() } stop(): void { @@ -138,8 +138,9 @@ export default class Combobox { clearSelection(): void { this.input.removeAttribute('aria-activedescendant') - for (const el of this.list.querySelectorAll('[aria-selected="true"]')) { + for (const el of this.list.querySelectorAll('[aria-selected="true"], [data-combobox-option-default="true"]')) { el.removeAttribute('aria-selected') + el.removeAttribute('data-combobox-option-default') } } From dbf90b53556067ff58182659e549150c2fd9033c Mon Sep 17 00:00:00 2001 From: Andrew Leach Date: Mon, 26 Feb 2024 17:11:40 +0000 Subject: [PATCH 5/5] Small test --- src/index.ts | 2 +- test/test.js | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/index.ts b/src/index.ts index 9c85298..35efe6e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -108,7 +108,7 @@ export default class Combobox { const focusIndex = els.indexOf(focusEl) if ((focusIndex === els.length - 1 && indexDiff === 1) || (focusIndex === 0 && indexDiff === -1)) { - this.resetSelection() + this.clearSelection() this.input.focus() return } diff --git a/test/test.js b/test/test.js index 0368285..a39d783 100644 --- a/test/test.js +++ b/test/test.js @@ -340,8 +340,6 @@ describe('combobox-nav', function () {
  • BB-8
  • Hubot
  • R2-D2
    • - -
  • Wall-E
  • Link
    • ` @@ -373,6 +371,23 @@ describe('combobox-nav', function () { assert.equal(list.children[0].getAttribute('aria-selected'), 'true') }) + it('pressing key down off the last item will have no items selected', () => { + // Get all visible options in the list + const options = document.querySelectorAll('[role=option]:not([aria-hidden=true])') + // Key press down for each item and ensure the next is selected + for (let i = 0; i < options.length; i++) { + if (i > 0) { + assert.equal(options[i - 1].getAttribute('aria-selected'), null) + } + + assert.equal(options[i].getAttribute('aria-selected'), 'true') + press(input, 'ArrowDown') + } + + const selected = document.querySelectorAll('[aria-selected]') + assert.equal(selected.length, 0) + }) + it('indicates first option when restarted', () => { combobox.stop() combobox.start()