From 791adef0ed5c1e018a56a15c866b534bd5b2944f Mon Sep 17 00:00:00 2001 From: David Graham Date: Fri, 8 Nov 2019 14:42:11 -0700 Subject: [PATCH 1/3] Bind details and menu to event listeners Removes null and type checking on event targets we know to be correct. --- index.js | 64 ++++++++++++++++++++++++++------------------------------ 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/index.js b/index.js index 9a550ca..3af3bb9 100644 --- a/index.js +++ b/index.js @@ -37,17 +37,26 @@ class DetailsMenuElement extends HTMLElement { if (!summary.hasAttribute('role')) summary.setAttribute('role', 'button') } - details.addEventListener('click', shouldCommit) - details.addEventListener('change', shouldCommit) - details.addEventListener('keydown', keydown) - details.addEventListener('toggle', loadFragment, {once: true}) - details.addEventListener('toggle', closeCurrentMenu) - if (this.preload) { - details.addEventListener('mouseover', loadFragment, {once: true}) + const subscriptions = [focusOnOpen(details)] + const state = { + details, + subscriptions, + loaded: false, + shouldCommit: shouldCommit.bind(null, details, this), + keydown: keydown.bind(null, details, this), + loadFragment: loadFragment.bind(null, details, this), + closeCurrentMenu: closeCurrentMenu.bind(null, details) } + states.set(this, state) - const subscriptions = [focusOnOpen(details)] - states.set(this, {details, subscriptions, loaded: false}) + details.addEventListener('click', state.shouldCommit) + details.addEventListener('change', state.shouldCommit) + details.addEventListener('keydown', state.keydown) + details.addEventListener('toggle', state.loadFragment, {once: true}) + details.addEventListener('toggle', state.closeCurrentMenu) + if (this.preload) { + details.addEventListener('mouseover', state.loadFragment, {once: true}) + } } disconnectedCallback() { @@ -60,24 +69,18 @@ class DetailsMenuElement extends HTMLElement { for (const sub of subscriptions) { sub.unsubscribe() } - details.removeEventListener('click', shouldCommit) - details.removeEventListener('change', shouldCommit) - details.removeEventListener('keydown', keydown) - details.removeEventListener('toggle', loadFragment, {once: true}) - details.removeEventListener('toggle', closeCurrentMenu) - details.removeEventListener('mouseover', loadFragment, {once: true}) + details.removeEventListener('click', state.shouldCommit) + details.removeEventListener('change', state.shouldCommit) + details.removeEventListener('keydown', state.keydown) + details.removeEventListener('toggle', state.loadFragment, {once: true}) + details.removeEventListener('toggle', state.closeCurrentMenu) + details.removeEventListener('mouseover', state.loadFragment, {once: true}) } } const states = new WeakMap() -function loadFragment(event: Event) { - const details = event.currentTarget - if (!(details instanceof Element)) return - - const menu = details.querySelector('details-menu') - if (!menu) return - +function loadFragment(details: Element, menu: DetailsMenuElement) { const src = menu.getAttribute('src') if (!src) return @@ -117,14 +120,12 @@ function focusOnOpen(details: Element) { } } -function closeCurrentMenu(event: Event) { - const el = event.currentTarget - if (!(el instanceof Element)) return - if (!el.hasAttribute('open')) return +function closeCurrentMenu(details: Element) { + if (!details.hasAttribute('open')) return for (const menu of document.querySelectorAll('details[open] > details-menu')) { const opened = menu.closest('details') - if (opened && opened !== el && !opened.contains(el)) { + if (opened && opened !== details && !opened.contains(details)) { opened.removeAttribute('open') } } @@ -163,13 +164,10 @@ function sibling(details: Element, next: boolean): ?HTMLElement { const ctrlBindings = navigator.userAgent.match(/Macintosh/) -function shouldCommit(event: Event) { +function shouldCommit(details: Element, menu: DetailsMenuElement, event: Event) { const target = event.target if (!(target instanceof Element)) return - const details = event.currentTarget - if (!(details instanceof Element)) return - // Ignore clicks from nested details. if (target.closest('details') !== details) return @@ -219,9 +217,7 @@ function commit(selected: Element, details: Element) { ) } -function keydown(event: KeyboardEvent) { - const details = event.currentTarget - if (!(details instanceof Element)) return +function keydown(details: Element, menu: DetailsMenuElement, event: KeyboardEvent) { const isSummaryFocused = event.target instanceof Element && event.target.tagName === 'SUMMARY' // Ignore key presses from nested details. From 37ee469de36d5916b592e8ceef24c613f698635e Mon Sep 17 00:00:00 2001 From: David Graham Date: Fri, 8 Nov 2019 15:02:11 -0700 Subject: [PATCH 2/3] Subscribe to events Consolidates adding and removing listeners into a subscription pattern. --- index.js | 62 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/index.js b/index.js index 3af3bb9..4a114d5 100644 --- a/index.js +++ b/index.js @@ -37,48 +37,47 @@ class DetailsMenuElement extends HTMLElement { if (!summary.hasAttribute('role')) summary.setAttribute('role', 'button') } - const subscriptions = [focusOnOpen(details)] - const state = { - details, - subscriptions, - loaded: false, - shouldCommit: shouldCommit.bind(null, details, this), - keydown: keydown.bind(null, details, this), - loadFragment: loadFragment.bind(null, details, this), - closeCurrentMenu: closeCurrentMenu.bind(null, details) - } - states.set(this, state) - - details.addEventListener('click', state.shouldCommit) - details.addEventListener('change', state.shouldCommit) - details.addEventListener('keydown', state.keydown) - details.addEventListener('toggle', state.loadFragment, {once: true}) - details.addEventListener('toggle', state.closeCurrentMenu) - if (this.preload) { - details.addEventListener('mouseover', state.loadFragment, {once: true}) - } + const subscriptions = [ + fromEvent(details, 'click', e => shouldCommit(details, this, e)), + fromEvent(details, 'change', e => shouldCommit(details, this, e)), + fromEvent(details, 'keydown', e => keydown(details, this, e)), + fromEvent(details, 'toggle', () => loadFragment(details, this), {once: true}), + fromEvent(details, 'toggle', () => closeCurrentMenu(details)), + this.preload + ? fromEvent(details, 'mouseover', () => loadFragment(details, this), {once: true}) + : NullSubscription, + focusOnOpen(details) + ] + + states.set(this, {subscriptions, loaded: false}) } disconnectedCallback() { const state = states.get(this) if (!state) return - states.delete(this) - - const {details, subscriptions} = state - for (const sub of subscriptions) { + for (const sub of state.subscriptions) { sub.unsubscribe() } - details.removeEventListener('click', state.shouldCommit) - details.removeEventListener('change', state.shouldCommit) - details.removeEventListener('keydown', state.keydown) - details.removeEventListener('toggle', state.loadFragment, {once: true}) - details.removeEventListener('toggle', state.closeCurrentMenu) - details.removeEventListener('mouseover', state.loadFragment, {once: true}) } } const states = new WeakMap() +const NullSubscription = {unsubscribe() {}} + +function fromEvent( + target: EventTarget, + eventName: string, + onNext: EventHandler, + options: EventListenerOptionsOrUseCapture = false +) { + target.addEventListener(eventName, onNext, options) + return { + unsubscribe: () => { + target.removeEventListener(eventName, onNext, options) + } + } +} function loadFragment(details: Element, menu: DetailsMenuElement) { const src = menu.getAttribute('src') @@ -217,7 +216,8 @@ function commit(selected: Element, details: Element) { ) } -function keydown(details: Element, menu: DetailsMenuElement, event: KeyboardEvent) { +function keydown(details: Element, menu: DetailsMenuElement, event: Event) { + if (!(event instanceof KeyboardEvent)) return const isSummaryFocused = event.target instanceof Element && event.target.tagName === 'SUMMARY' // Ignore key presses from nested details. From 82ac98453c41b83bdd6b8eb8fd6b31ffc4f665c2 Mon Sep 17 00:00:00 2001 From: David Graham Date: Fri, 8 Nov 2019 15:11:12 -0700 Subject: [PATCH 3/3] Use fromEvent in autofocus listeners --- index.js | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/index.js b/index.js index 4a114d5..07c996b 100644 --- a/index.js +++ b/index.js @@ -46,7 +46,7 @@ class DetailsMenuElement extends HTMLElement { this.preload ? fromEvent(details, 'mouseover', () => loadFragment(details, this), {once: true}) : NullSubscription, - focusOnOpen(details) + ...focusOnOpen(details) ] states.set(this, {subscriptions, loaded: false}) @@ -63,6 +63,8 @@ class DetailsMenuElement extends HTMLElement { } const states = new WeakMap() + +type Subscription = {unsubscribe(): void} const NullSubscription = {unsubscribe() {}} function fromEvent( @@ -70,7 +72,7 @@ function fromEvent( eventName: string, onNext: EventHandler, options: EventListenerOptionsOrUseCapture = false -) { +): Subscription { target.addEventListener(eventName, onNext, options) return { unsubscribe: () => { @@ -96,7 +98,7 @@ function loadFragment(details: Element, menu: DetailsMenuElement) { } } -function focusOnOpen(details: Element) { +function focusOnOpen(details: Element): Array { let isMouse = false const onmousedown = () => (isMouse = true) const onkeydown = () => (isMouse = false) @@ -106,17 +108,11 @@ function focusOnOpen(details: Element) { if (!isMouse) focusFirstItem(details) } - details.addEventListener('mousedown', onmousedown) - details.addEventListener('keydown', onkeydown) - details.addEventListener('toggle', ontoggle) - - return { - unsubscribe: () => { - details.removeEventListener('mousedown', onmousedown) - details.removeEventListener('keydown', onkeydown) - details.removeEventListener('toggle', ontoggle) - } - } + return [ + fromEvent(details, 'mousedown', onmousedown), + fromEvent(details, 'keydown', onkeydown), + fromEvent(details, 'toggle', ontoggle) + ] } function closeCurrentMenu(details: Element) {