From c27d30d6d7dbbb35a43cfd839ed7c0b627165ecc Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Fri, 4 Jan 2019 17:22:39 -0500 Subject: [PATCH 1/7] Replace keyboard binding code with combobox-nav --- examples/index.html | 2 +- package-lock.json | 30 +++++++++++++++++ package.json | 6 +++- rollup.config.js | 4 +++ src/autocomplete.js | 82 +++++++++------------------------------------ test/test.js | 4 +-- 6 files changed, 58 insertions(+), 70 deletions(-) diff --git a/examples/index.html b/examples/index.html index d1e140e..4b693e5 100644 --- a/examples/index.html +++ b/examples/index.html @@ -41,7 +41,7 @@
- +
    diff --git a/package-lock.json b/package-lock.json index b848b5a..e852c88 100644 --- a/package-lock.json +++ b/package-lock.json @@ -185,6 +185,11 @@ } } }, + "@github/combobox-nav": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@github/combobox-nav/-/combobox-nav-0.2.0.tgz", + "integrity": "sha512-hdxFUlDZD8PAjtXSXBmwBlDZrp+iMm+k1kb9XoIf9tsqGUh3uwh/SnUDoXXCpmui1xvLJUFB6+2q67LG2Fgagw==" + }, "@types/estree": { "version": "0.0.39", "resolved": "https://registry.npmjs.org/@types/estree/-/estree-0.0.39.tgz", @@ -3711,6 +3716,12 @@ "is-extglob": "^1.0.0" } }, + "is-module": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/is-module/-/is-module-1.0.0.tgz", + "integrity": "sha1-Mlj7afeMFNW4FdZkM2tM/7ZEFZE=", + "dev": true + }, "is-number": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/is-number/-/is-number-2.1.0.tgz", @@ -5611,6 +5622,25 @@ "rollup-pluginutils": "^1.5.0" } }, + "rollup-plugin-node-resolve": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/rollup-plugin-node-resolve/-/rollup-plugin-node-resolve-4.0.0.tgz", + "integrity": "sha512-7Ni+/M5RPSUBfUaP9alwYQiIKnKeXCOHiqBpKUl9kwp3jX5ZJtgXAait1cne6pGEVUUztPD6skIKH9Kq9sNtfw==", + "dev": true, + "requires": { + "builtin-modules": "^3.0.0", + "is-module": "^1.0.0", + "resolve": "^1.8.1" + }, + "dependencies": { + "builtin-modules": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/builtin-modules/-/builtin-modules-3.0.0.tgz", + "integrity": "sha512-hMIeU4K2ilbXV6Uv93ZZ0Avg/M91RaKXucQ+4me2Do1txxBDyDZWCBa5bJSLqoNTRpXTLwEzIk1KmloenDDjhg==", + "dev": true + } + } + }, "rollup-pluginutils": { "version": "1.5.2", "resolved": "https://registry.npmjs.org/rollup-pluginutils/-/rollup-pluginutils-1.5.2.tgz", diff --git a/package.json b/package.json index 63de5d9..b653098 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,10 @@ "karma-mocha-reporter": "^2.2.5", "mocha": "^5.2.0", "rollup": "^0.64.1", - "rollup-plugin-babel": "^3.0.7" + "rollup-plugin-babel": "^3.0.7", + "rollup-plugin-node-resolve": "^4.0.0" + }, + "dependencies": { + "@github/combobox-nav": "^0.2.0" } } diff --git a/rollup.config.js b/rollup.config.js index ba36bfb..cff7aef 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -1,6 +1,7 @@ /* @flow */ import babel from 'rollup-plugin-babel' +import resolve from 'rollup-plugin-node-resolve' const pkg = require('./package.json') @@ -18,6 +19,9 @@ export default { } ], plugins: [ + resolve({ + main: true + }), babel({ plugins: ['transform-custom-element-classes'], presets: ['es2015-rollup', 'flow'] diff --git a/src/autocomplete.js b/src/autocomplete.js index d707509..f36f7bc 100644 --- a/src/autocomplete.js +++ b/src/autocomplete.js @@ -4,8 +4,7 @@ import type AutocompleteElement from './auto-complete-element' import debounce from './debounce' import {fragment} from './send' import {scrollTo} from './scroll' - -const ctrlBindings = navigator.userAgent.match(/Macintosh/) +import {install as installCombobox, uninstall as uninstallCombobox} from '@github/combobox-nav' export default class Autocomplete { container: AutocompleteElement @@ -13,11 +12,11 @@ export default class Autocomplete { results: HTMLElement onInputChange: () => void - onResultsClick: MouseEvent => void onResultsMouseDown: () => void onInputBlur: () => void onInputFocus: () => void onKeydown: KeyboardEvent => void + onCommit: Event => void mouseDown: boolean @@ -33,18 +32,20 @@ export default class Autocomplete { this.mouseDown = false this.onInputChange = debounce(this.onInputChange.bind(this), 300) - this.onResultsClick = this.onResultsClick.bind(this) this.onResultsMouseDown = this.onResultsMouseDown.bind(this) this.onInputBlur = this.onInputBlur.bind(this) this.onInputFocus = this.onInputFocus.bind(this) this.onKeydown = this.onKeydown.bind(this) + this.onCommit = this.onCommit.bind(this) this.input.addEventListener('keydown', this.onKeydown) this.input.addEventListener('focus', this.onInputFocus) this.input.addEventListener('blur', this.onInputBlur) this.input.addEventListener('input', this.onInputChange) this.results.addEventListener('mousedown', this.onResultsMouseDown) - this.results.addEventListener('click', this.onResultsClick) + this.results.addEventListener('combobox-commit', this.onCommit) + + installCombobox(this.input, this.results) } destroy() { @@ -53,7 +54,9 @@ export default class Autocomplete { this.input.removeEventListener('blur', this.onInputBlur) this.input.removeEventListener('input', this.onInputChange) this.results.removeEventListener('mousedown', this.onResultsMouseDown) - this.results.removeEventListener('click', this.onResultsClick) + this.results.removeEventListener('combobox-commit', this.onCommit) + + uninstallCombobox(this.input, this.results) } sibling(next: boolean): HTMLElement { @@ -75,59 +78,10 @@ export default class Autocomplete { } onKeydown(event: KeyboardEvent) { - switch (event.key) { - case 'Escape': - if (this.container.open) { - this.container.open = false - event.stopPropagation() - event.preventDefault() - } - break - case 'ArrowDown': - { - const item = this.sibling(true) - if (item) this.select(item) - event.preventDefault() - } - break - case 'ArrowUp': - { - const item = this.sibling(false) - if (item) this.select(item) - event.preventDefault() - } - break - case 'n': - if (ctrlBindings && event.ctrlKey) { - const item = this.sibling(true) - if (item) this.select(item) - event.preventDefault() - } - break - case 'p': - if (ctrlBindings && event.ctrlKey) { - const item = this.sibling(false) - if (item) this.select(item) - event.preventDefault() - } - break - case 'Tab': - { - const selected = this.results.querySelector('[aria-selected="true"]') - if (selected) { - this.commit(selected) - } - } - break - case 'Enter': - { - const selected = this.results.querySelector('[aria-selected="true"]') - if (selected && this.container.open) { - this.commit(selected) - event.preventDefault() - } - } - break + if (event.key === 'Escape' && this.container.open) { + this.container.open = false + event.stopPropagation() + event.preventDefault() } } @@ -140,7 +94,9 @@ export default class Autocomplete { this.container.open = false } - commit(selected: Element) { + onCommit({target}: Event) { + const selected = target + if (!(selected instanceof HTMLElement)) return if (selected.getAttribute('aria-disabled') === 'true') return if (selected instanceof HTMLAnchorElement) { @@ -154,12 +110,6 @@ export default class Autocomplete { this.container.open = false } - onResultsClick(event: MouseEvent) { - if (!(event.target instanceof Element)) return - const selected = event.target.closest('[role="option"]') - if (selected) this.commit(selected) - } - onResultsMouseDown() { this.mouseDown = true this.results.addEventListener('mouseup', () => (this.mouseDown = false), {once: true}) diff --git a/test/test.js b/test/test.js index dab0ff9..7663682 100644 --- a/test/test.js +++ b/test/test.js @@ -16,7 +16,7 @@ describe('auto-complete element', function() { document.body.innerHTML = `
    - +
    @@ -131,7 +131,7 @@ function keydown(element, key) { return '' }) e.key = key - if (key !== key.toLowerCase()) e.shiftKey = true + if (key.length === 1 && key !== key.toLowerCase()) e.shiftKey = true e.keyCode = keyCodes[key] || key.toUpperCase().charCodeAt(0) e.which = e.keyCode From 2c9f896c24e2627db465f1d3babc021e4b46f74c Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Fri, 4 Jan 2019 17:28:48 -0500 Subject: [PATCH 2/7] Patch aria-owns attribute for backwards compatibility --- examples/index.html | 2 +- src/auto-complete-element.js | 1 + test/test.js | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/index.html b/examples/index.html index 4b693e5..d1e140e 100644 --- a/examples/index.html +++ b/examples/index.html @@ -41,7 +41,7 @@ - +
      diff --git a/src/auto-complete-element.js b/src/auto-complete-element.js index f82f9c1..496eb81 100644 --- a/src/auto-complete-element.js +++ b/src/auto-complete-element.js @@ -16,6 +16,7 @@ export default class AutocompleteElement extends HTMLElement { const input = this.querySelector('input') const results = document.getElementById(owns) if (!(input instanceof HTMLInputElement) || !results) return + input.setAttribute('aria-owns', owns) state.set(this, new Autocomplete(this, input, results)) this.setAttribute('role', 'combobox') diff --git a/test/test.js b/test/test.js index 7663682..34a5a3f 100644 --- a/test/test.js +++ b/test/test.js @@ -16,7 +16,7 @@ describe('auto-complete element', function() { document.body.innerHTML = `
      - +
      From a08610f756a39425bf21dddbec96967f1bbd295a Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Mon, 7 Jan 2019 11:29:44 -0500 Subject: [PATCH 3/7] Add test to ensure disabled items don't get committed --- test/karma.config.js | 1 + test/test.js | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/test/karma.config.js b/test/karma.config.js index 0364f64..6f0737c 100644 --- a/test/karma.config.js +++ b/test/karma.config.js @@ -5,6 +5,7 @@ function completer(request, response, next) {
    • first
    • second
    • third
      • +
    • fourth
      • link `) return diff --git a/test/test.js b/test/test.js index 34a5a3f..90e2b18 100644 --- a/test/test.js +++ b/test/test.js @@ -31,7 +31,7 @@ describe('auto-complete element', function() { triggerInput(input, 'hub') await once(container, 'loadend') - assert.equal(4, popup.children.length) + assert.equal(5, popup.children.length) }) it('respects arrow keys', async function() { @@ -67,6 +67,24 @@ describe('auto-complete element', function() { assert.isFalse(container.open) }) + it('does not commit on disabled option', async function() { + const container = document.querySelector('auto-complete') + const input = container.querySelector('input') + const popup = container.querySelector('#popup') + + triggerInput(input, 'hub') + await once(container, 'loadend') + + assert.isFalse(keydown(input, 'ArrowDown')) + assert.isFalse(keydown(input, 'ArrowDown')) + assert.isFalse(keydown(input, 'ArrowDown')) + assert.isFalse(keydown(input, 'ArrowDown')) + assert.equal('fourth', popup.querySelector('[aria-selected="true"]').textContent) + assert.isFalse(keydown(input, 'Enter')) + assert.equal('', container.value) + assert.isTrue(container.open) + }) + it('does not commit text value on link item navigation', async function() { const container = document.querySelector('auto-complete') const input = container.querySelector('input') From 1634d65daaf513c78747a11421ee4ddde9475267 Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Mon, 7 Jan 2019 11:31:21 -0500 Subject: [PATCH 4/7] Ensure build breaks --- src/autocomplete.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/autocomplete.js b/src/autocomplete.js index f36f7bc..3aa125f 100644 --- a/src/autocomplete.js +++ b/src/autocomplete.js @@ -97,7 +97,6 @@ export default class Autocomplete { onCommit({target}: Event) { const selected = target if (!(selected instanceof HTMLElement)) return - if (selected.getAttribute('aria-disabled') === 'true') return if (selected instanceof HTMLAnchorElement) { selected.click() From 91488ce3bff3cc7582a1d596a54779b7ff37d3ac Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Mon, 7 Jan 2019 11:35:33 -0500 Subject: [PATCH 5/7] Upgrade combobox-nav and fix build --- package-lock.json | 6 +++--- package.json | 2 +- test/test.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index e852c88..6199412 100644 --- a/package-lock.json +++ b/package-lock.json @@ -186,9 +186,9 @@ } }, "@github/combobox-nav": { - "version": "0.2.0", - "resolved": "https://registry.npmjs.org/@github/combobox-nav/-/combobox-nav-0.2.0.tgz", - "integrity": "sha512-hdxFUlDZD8PAjtXSXBmwBlDZrp+iMm+k1kb9XoIf9tsqGUh3uwh/SnUDoXXCpmui1xvLJUFB6+2q67LG2Fgagw==" + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/@github/combobox-nav/-/combobox-nav-0.2.1.tgz", + "integrity": "sha512-lyyhubgzLe+3kzwJF9CklIZeVxMn3CwnM0Q3KSvQPxKMeTWqcPLByzIffG/0Ulp/Z9kn3mZISoNMfwFq4Uj0mg==" }, "@types/estree": { "version": "0.0.39", diff --git a/package.json b/package.json index b653098..f92f70a 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,6 @@ "rollup-plugin-node-resolve": "^4.0.0" }, "dependencies": { - "@github/combobox-nav": "^0.2.0" + "@github/combobox-nav": "^0.2.1" } } diff --git a/test/test.js b/test/test.js index 90e2b18..4bac24b 100644 --- a/test/test.js +++ b/test/test.js @@ -80,7 +80,7 @@ describe('auto-complete element', function() { assert.isFalse(keydown(input, 'ArrowDown')) assert.isFalse(keydown(input, 'ArrowDown')) assert.equal('fourth', popup.querySelector('[aria-selected="true"]').textContent) - assert.isFalse(keydown(input, 'Enter')) + assert.isTrue(keydown(input, 'Enter')) assert.equal('', container.value) assert.isTrue(container.open) }) From 521668d7eb44d7859de562e4d85159b9d0d59e09 Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Mon, 14 Jan 2019 17:39:01 -0500 Subject: [PATCH 6/7] Upgrade comobobox-nav to ensure click on disabled item is cancelled --- examples/index.html | 4 ++-- package-lock.json | 6 +++--- package.json | 2 +- test/test.js | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/index.html b/examples/index.html index d1e140e..c191055 100644 --- a/examples/index.html +++ b/examples/index.html @@ -46,7 +46,7 @@ - - + + diff --git a/package-lock.json b/package-lock.json index b7c9e59..977e722 100644 --- a/package-lock.json +++ b/package-lock.json @@ -981,9 +981,9 @@ } }, "@github/combobox-nav": { - "version": "0.2.1", - "resolved": "https://registry.npmjs.org/@github/combobox-nav/-/combobox-nav-0.2.1.tgz", - "integrity": "sha512-lyyhubgzLe+3kzwJF9CklIZeVxMn3CwnM0Q3KSvQPxKMeTWqcPLByzIffG/0Ulp/Z9kn3mZISoNMfwFq4Uj0mg==" + "version": "0.2.2", + "resolved": "https://registry.npmjs.org/@github/combobox-nav/-/combobox-nav-0.2.2.tgz", + "integrity": "sha512-R+6uJtDvkciRarqpkOS7ykeY7fLl5d7EbyfZm0Qdxs6qi4ppVkIkOjSQApG8Paz6KHH2iYBYu4Sc0Gagz0EhYw==" }, "@types/estree": { "version": "0.0.39", diff --git a/package.json b/package.json index 693f454..04bdd20 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,6 @@ "rollup-plugin-node-resolve": "^4.0.0" }, "dependencies": { - "@github/combobox-nav": "^0.2.1" + "@github/combobox-nav": "^0.2.2" } } diff --git a/test/test.js b/test/test.js index 4bac24b..90e2b18 100644 --- a/test/test.js +++ b/test/test.js @@ -80,7 +80,7 @@ describe('auto-complete element', function() { assert.isFalse(keydown(input, 'ArrowDown')) assert.isFalse(keydown(input, 'ArrowDown')) assert.equal('fourth', popup.querySelector('[aria-selected="true"]').textContent) - assert.isTrue(keydown(input, 'Enter')) + assert.isFalse(keydown(input, 'Enter')) assert.equal('', container.value) assert.isTrue(container.open) }) From 7b059504238cb648ade2f1e2f79805d8526c6e81 Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Tue, 15 Jan 2019 13:49:13 -0500 Subject: [PATCH 7/7] Undo comment --- examples/index.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/index.html b/examples/index.html index c191055..d1e140e 100644 --- a/examples/index.html +++ b/examples/index.html @@ -46,7 +46,7 @@ - - + +