From 7947eb7459d5230103ec2fb230b755d0ce8de9e5 Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 7 Jun 2019 08:54:03 +0100 Subject: [PATCH 1/7] focus middleware --- src/core/middleware/focus.ts | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/core/middleware/focus.ts diff --git a/src/core/middleware/focus.ts b/src/core/middleware/focus.ts new file mode 100644 index 000000000..a970e614e --- /dev/null +++ b/src/core/middleware/focus.ts @@ -0,0 +1,38 @@ +import { create, diffProperties, invalidator } from '../vdom'; +import { cache } from './cache'; + +export interface FocusProperties { + focus?: (() => boolean); +} + +const factory = create({ cache, diffProperties, invalidator }).properties(); + +export const focus = factory(({ middleware: { cache, diffProperties, invalidator } }) => { + cache.set('current', 0); + cache.set('previous', 0); + diffProperties((_: FocusProperties, next: FocusProperties) => { + const result = next.focus && next.focus(); + if (result) { + let current = cache.get('current') || 0; + current++; + cache.set('current', current); + invalidator(); + } + }); + return { + shouldFocus(): boolean { + const current = cache.get('current') || 0; + const previous = cache.get('previous') || 0; + cache.set('previous', current); + return current !== previous; + }, + focus(): void { + let current = cache.get('current') || 0; + current++; + cache.set('current', current); + invalidator(); + } + }; +}); + +export default focus; From d3308b89a45abe94b424657c842867c6733f6130 Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 14 Jun 2019 09:52:47 +0100 Subject: [PATCH 2/7] Add unit tests and tweak focus middleware --- src/core/middleware/focus.ts | 28 ++++------- tests/core/unit/middleware/focus.ts | 74 +++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 18 deletions(-) create mode 100644 tests/core/unit/middleware/focus.ts diff --git a/src/core/middleware/focus.ts b/src/core/middleware/focus.ts index a970e614e..1989b1ac3 100644 --- a/src/core/middleware/focus.ts +++ b/src/core/middleware/focus.ts @@ -1,36 +1,28 @@ -import { create, diffProperties, invalidator } from '../vdom'; +import { create, diffProperty } from '../vdom'; +import { icache } from './icache'; import { cache } from './cache'; +import { FocusProperties } from '../mixins/Focus'; -export interface FocusProperties { - focus?: (() => boolean); -} +const factory = create({ icache, cache, diffProperty }).properties(); -const factory = create({ cache, diffProperties, invalidator }).properties(); - -export const focus = factory(({ middleware: { cache, diffProperties, invalidator } }) => { - cache.set('current', 0); - cache.set('previous', 0); - diffProperties((_: FocusProperties, next: FocusProperties) => { +export const focus = factory(({ middleware: { icache, cache, diffProperty } }) => { + diffProperty('focus', (_: FocusProperties, next: FocusProperties) => { const result = next.focus && next.focus(); if (result) { - let current = cache.get('current') || 0; - current++; - cache.set('current', current); - invalidator(); + let current = icache.get('current') || 0; + icache.set('current', current + 1); } }); return { shouldFocus(): boolean { - const current = cache.get('current') || 0; + const current = icache.get('current') || 0; const previous = cache.get('previous') || 0; cache.set('previous', current); return current !== previous; }, focus(): void { let current = cache.get('current') || 0; - current++; - cache.set('current', current); - invalidator(); + icache.set('current', current + 1); } }; }); diff --git a/tests/core/unit/middleware/focus.ts b/tests/core/unit/middleware/focus.ts new file mode 100644 index 000000000..0811c97e9 --- /dev/null +++ b/tests/core/unit/middleware/focus.ts @@ -0,0 +1,74 @@ +const { it, describe, afterEach } = intern.getInterface('bdd'); +const { assert } = intern.getPlugin('chai'); +import { sandbox } from 'sinon'; + +import focusMiddleware from '../../../../src/core/middleware/focus'; +import cacheMiddleware from '../../../../src/core/middleware/cache'; +import icacheMiddleware from '../../../../src/core/middleware/icache'; + +const sb = sandbox.create(); +const diffPropertyStub = sb.stub(); + +function cacheFactory() { + return cacheMiddleware().callback({ id: 'test-cache', properties: {}, middleware: { destroy: sb.stub() } }); +} + +function icacheFactory() { + return icacheMiddleware().callback({ + id: 'test-cache', + properties: {}, + middleware: { cache: cacheFactory(), invalidator: sb.stub() } + }); +} + +describe('focus middleware', () => { + afterEach(() => { + sb.resetHistory(); + }); + + it('`shouldFocus` is controlled by calls to `focus`', () => { + const { callback } = focusMiddleware(); + const focus = callback({ + id: 'test', + middleware: { + diffProperty: diffPropertyStub, + cache: cacheFactory(), + icache: icacheFactory() + }, + properties: {} + }); + assert.isFalse(focus.shouldFocus()); + focus.focus(); + assert.isTrue(focus.shouldFocus()); + }); + + it('`shouldFocus` returns true when focus property returns true', () => { + const { callback } = focusMiddleware(); + const focus = callback({ + id: 'test', + middleware: { + diffProperty: diffPropertyStub, + cache: cacheFactory(), + icache: icacheFactory() + }, + properties: {} + }); + diffPropertyStub.getCall(0).callArgWith(1, {}, { focus: () => true }); + assert.isTrue(focus.shouldFocus()); + }); + + it('`shouldFocus` returns false when focus property returns false', () => { + const { callback } = focusMiddleware(); + const focus = callback({ + id: 'test', + middleware: { + diffProperty: diffPropertyStub, + cache: cacheFactory(), + icache: icacheFactory() + }, + properties: {} + }); + diffPropertyStub.getCall(0).callArgWith(1, {}, { focus: () => false }); + assert.isFalse(focus.shouldFocus()); + }); +}); From c2c59e44cdf2244b94aa5f2255327b5a77294ef7 Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 14 Jun 2019 10:28:40 +0100 Subject: [PATCH 3/7] Add to all.ts --- tests/core/unit/middleware/all.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/core/unit/middleware/all.ts b/tests/core/unit/middleware/all.ts index b787ca123..8c1051112 100644 --- a/tests/core/unit/middleware/all.ts +++ b/tests/core/unit/middleware/all.ts @@ -2,6 +2,7 @@ import './block'; import './cache'; import './dimensions'; import './i18n'; +import './focus'; import './icache'; import './injector'; import './intersection'; From 34127618e4c78436c87711cd037c9f1f6f6547f3 Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 14 Jun 2019 14:03:56 +0100 Subject: [PATCH 4/7] Add isFocused API to determine if a node has focus --- src/core/middleware/focus.ts | 27 +++++++- tests/core/unit/middleware/focus.ts | 101 ++++++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 8 deletions(-) diff --git a/src/core/middleware/focus.ts b/src/core/middleware/focus.ts index 1989b1ac3..5fe101bc2 100644 --- a/src/core/middleware/focus.ts +++ b/src/core/middleware/focus.ts @@ -1,11 +1,13 @@ -import { create, diffProperty } from '../vdom'; +import global from '../../shim/global'; +import { create, diffProperty, node, destroy, invalidator } from '../vdom'; import { icache } from './icache'; import { cache } from './cache'; import { FocusProperties } from '../mixins/Focus'; -const factory = create({ icache, cache, diffProperty }).properties(); +const factory = create({ icache, cache, diffProperty, node, destroy, invalidator }).properties(); -export const focus = factory(({ middleware: { icache, cache, diffProperty } }) => { +export const focus = factory(({ middleware: { icache, cache, diffProperty, node, destroy, invalidator } }) => { + let initialized = false; diffProperty('focus', (_: FocusProperties, next: FocusProperties) => { const result = next.focus && next.focus(); if (result) { @@ -13,6 +15,13 @@ export const focus = factory(({ middleware: { icache, cache, diffProperty } }) = icache.set('current', current + 1); } }); + function onFocusChange() { + invalidator(); + } + destroy(() => { + global.document.removeEventListener('focusin', onFocusChange); + global.document.removeEventListener('focusout', onFocusChange); + }); return { shouldFocus(): boolean { const current = icache.get('current') || 0; @@ -23,6 +32,18 @@ export const focus = factory(({ middleware: { icache, cache, diffProperty } }) = focus(): void { let current = cache.get('current') || 0; icache.set('current', current + 1); + }, + isFocused(key: string | number): boolean { + const domNode = node.get(key); + if (!domNode) { + return false; + } + if (!initialized) { + global.document.addEventListener('focusin', onFocusChange); + global.document.addEventListener('focusout', onFocusChange); + initialized = true; + } + return global.document.activeElement === domNode; } }; }); diff --git a/tests/core/unit/middleware/focus.ts b/tests/core/unit/middleware/focus.ts index 0811c97e9..331764ca7 100644 --- a/tests/core/unit/middleware/focus.ts +++ b/tests/core/unit/middleware/focus.ts @@ -1,6 +1,8 @@ -const { it, describe, afterEach } = intern.getInterface('bdd'); +import global from '../../../../src/shim/global'; +const { it, afterEach } = intern.getInterface('bdd'); +const { describe } = intern.getPlugin('jsdom'); const { assert } = intern.getPlugin('chai'); -import { sandbox } from 'sinon'; +import { sandbox, SinonStub } from 'sinon'; import focusMiddleware from '../../../../src/core/middleware/focus'; import cacheMiddleware from '../../../../src/core/middleware/cache'; @@ -8,6 +10,15 @@ import icacheMiddleware from '../../../../src/core/middleware/icache'; const sb = sandbox.create(); const diffPropertyStub = sb.stub(); +const destroyStub = sb.stub(); +const nodeStub = { + get: sb.stub() +}; +const invalidatorStub = sb.stub(); + +function waitForCall(calls = 1, stub: SinonStub) { + return new Promise((res) => stub.onCall(calls - 1).callsFake(res)); +} function cacheFactory() { return cacheMiddleware().callback({ id: 'test-cache', properties: {}, middleware: { destroy: sb.stub() } }); @@ -33,7 +44,10 @@ describe('focus middleware', () => { middleware: { diffProperty: diffPropertyStub, cache: cacheFactory(), - icache: icacheFactory() + icache: icacheFactory(), + destroy: destroyStub, + node: nodeStub, + invalidator: invalidatorStub }, properties: {} }); @@ -49,7 +63,10 @@ describe('focus middleware', () => { middleware: { diffProperty: diffPropertyStub, cache: cacheFactory(), - icache: icacheFactory() + icache: icacheFactory(), + destroy: destroyStub, + node: nodeStub, + invalidator: invalidatorStub }, properties: {} }); @@ -64,11 +81,85 @@ describe('focus middleware', () => { middleware: { diffProperty: diffPropertyStub, cache: cacheFactory(), - icache: icacheFactory() + icache: icacheFactory(), + destroy: destroyStub, + node: nodeStub, + invalidator: invalidatorStub }, properties: {} }); diffPropertyStub.getCall(0).callArgWith(1, {}, { focus: () => false }); assert.isFalse(focus.shouldFocus()); }); + + it('Should return false if the node is not available', () => { + const { callback } = focusMiddleware(); + const focus = callback({ + id: 'test', + middleware: { + diffProperty: diffPropertyStub, + cache: cacheFactory(), + icache: icacheFactory(), + destroy: destroyStub, + node: nodeStub, + invalidator: invalidatorStub + }, + properties: {} + }); + assert.isFalse(focus.isFocused('root')); + }); + + it('Should return true if the node is the active element', async () => { + const { callback } = focusMiddleware(); + const focus = callback({ + id: 'test', + middleware: { + diffProperty: diffPropertyStub, + cache: cacheFactory(), + icache: icacheFactory(), + destroy: destroyStub, + node: nodeStub, + invalidator: invalidatorStub + }, + properties: {} + }); + const div = global.document.createElement('div'); + const buttonOne = global.document.createElement('button'); + const buttonTwo = global.document.createElement('button'); + div.appendChild(buttonOne); + div.appendChild(buttonTwo); + global.document.body.appendChild(div); + const firstWait = waitForCall(1, invalidatorStub); + const secondWait = waitForCall(2, invalidatorStub); + const addEventListenerSpy = sb.spy(global.document, 'addEventListener'); + const removeEventListener = sb.spy(global.document, 'removeEventListener'); + nodeStub.get.withArgs('root').returns(buttonOne); + assert.isFalse(focus.isFocused('root')); + buttonOne.focus(); + if (global.fakeActiveElement) { + const activeStub = sb.stub(global, 'fakeActiveElement'); + activeStub + .onFirstCall() + .returns(buttonOne) + .onSecondCall() + .returns(buttonTwo); + } + let focusEvent = global.document.createEvent('Event'); + focusEvent.initEvent('focusin', true, true); + global.document.dispatchEvent(focusEvent); + await firstWait; + assert.isTrue(invalidatorStub.calledOnce); + assert.isTrue(focus.isFocused('root')); + assert.isTrue(addEventListenerSpy.calledTwice); + buttonTwo.focus(); + focusEvent = global.document.createEvent('Event'); + focusEvent.initEvent('focusin', true, true); + global.document.dispatchEvent(focusEvent); + await secondWait; + assert.isTrue(invalidatorStub.calledTwice); + assert.isFalse(focus.isFocused('root')); + assert.isTrue(addEventListenerSpy.calledTwice); + destroyStub.getCall(0).callArg(0); + assert.isTrue(removeEventListener.calledTwice); + }); }); From b91f603f11c8e8bea36d54bef5f5bb2d77053add Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 14 Jun 2019 15:05:42 +0100 Subject: [PATCH 5/7] Only invalidate if the previous or current focussed node is of interest --- src/core/middleware/focus.ts | 10 +++++++++- tests/core/unit/middleware/focus.ts | 25 +++++++++++++++---------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/core/middleware/focus.ts b/src/core/middleware/focus.ts index 5fe101bc2..8b81ec3f0 100644 --- a/src/core/middleware/focus.ts +++ b/src/core/middleware/focus.ts @@ -8,6 +8,7 @@ const factory = create({ icache, cache, diffProperty, node, destroy, invalidator export const focus = factory(({ middleware: { icache, cache, diffProperty, node, destroy, invalidator } }) => { let initialized = false; + const nodeSet = new Set(); diffProperty('focus', (_: FocusProperties, next: FocusProperties) => { const result = next.focus && next.focus(); if (result) { @@ -16,11 +17,17 @@ export const focus = factory(({ middleware: { icache, cache, diffProperty, node, } }); function onFocusChange() { - invalidator(); + const currentElement = cache.get('active-element'); + const activeElement = global.document.activeElement; + if (nodeSet.has(currentElement) || nodeSet.has(activeElement)) { + invalidator(); + } + cache.set('active-element', activeElement); } destroy(() => { global.document.removeEventListener('focusin', onFocusChange); global.document.removeEventListener('focusout', onFocusChange); + nodeSet.clear(); }); return { shouldFocus(): boolean { @@ -38,6 +45,7 @@ export const focus = factory(({ middleware: { icache, cache, diffProperty, node, if (!domNode) { return false; } + nodeSet.add(domNode); if (!initialized) { global.document.addEventListener('focusin', onFocusChange); global.document.addEventListener('focusout', onFocusChange); diff --git a/tests/core/unit/middleware/focus.ts b/tests/core/unit/middleware/focus.ts index 331764ca7..782fa0968 100644 --- a/tests/core/unit/middleware/focus.ts +++ b/tests/core/unit/middleware/focus.ts @@ -2,7 +2,7 @@ import global from '../../../../src/shim/global'; const { it, afterEach } = intern.getInterface('bdd'); const { describe } = intern.getPlugin('jsdom'); const { assert } = intern.getPlugin('chai'); -import { sandbox, SinonStub } from 'sinon'; +import { sandbox } from 'sinon'; import focusMiddleware from '../../../../src/core/middleware/focus'; import cacheMiddleware from '../../../../src/core/middleware/cache'; @@ -16,10 +16,6 @@ const nodeStub = { }; const invalidatorStub = sb.stub(); -function waitForCall(calls = 1, stub: SinonStub) { - return new Promise((res) => stub.onCall(calls - 1).callsFake(res)); -} - function cacheFactory() { return cacheMiddleware().callback({ id: 'test-cache', properties: {}, middleware: { destroy: sb.stub() } }); } @@ -126,11 +122,11 @@ describe('focus middleware', () => { const div = global.document.createElement('div'); const buttonOne = global.document.createElement('button'); const buttonTwo = global.document.createElement('button'); + const buttonThree = global.document.createElement('button'); div.appendChild(buttonOne); div.appendChild(buttonTwo); + div.appendChild(buttonThree); global.document.body.appendChild(div); - const firstWait = waitForCall(1, invalidatorStub); - const secondWait = waitForCall(2, invalidatorStub); const addEventListenerSpy = sb.spy(global.document, 'addEventListener'); const removeEventListener = sb.spy(global.document, 'removeEventListener'); nodeStub.get.withArgs('root').returns(buttonOne); @@ -142,12 +138,17 @@ describe('focus middleware', () => { .onFirstCall() .returns(buttonOne) .onSecondCall() - .returns(buttonTwo); + .returns(buttonOne) + .onThirdCall() + .returns(buttonTwo) + .onCall(3) + .returns(buttonTwo) + .onCall(4) + .returns(buttonThree); } let focusEvent = global.document.createEvent('Event'); focusEvent.initEvent('focusin', true, true); global.document.dispatchEvent(focusEvent); - await firstWait; assert.isTrue(invalidatorStub.calledOnce); assert.isTrue(focus.isFocused('root')); assert.isTrue(addEventListenerSpy.calledTwice); @@ -155,10 +156,14 @@ describe('focus middleware', () => { focusEvent = global.document.createEvent('Event'); focusEvent.initEvent('focusin', true, true); global.document.dispatchEvent(focusEvent); - await secondWait; assert.isTrue(invalidatorStub.calledTwice); assert.isFalse(focus.isFocused('root')); assert.isTrue(addEventListenerSpy.calledTwice); + buttonThree.focus(); + focusEvent = global.document.createEvent('Event'); + focusEvent.initEvent('focusin', true, true); + global.document.dispatchEvent(focusEvent); + assert.isTrue(invalidatorStub.calledTwice); destroyStub.getCall(0).callArg(0); assert.isTrue(removeEventListener.calledTwice); }); From fd1266dfd885f83b0b729858f3e2e839b6a235c9 Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 14 Jun 2019 15:07:11 +0100 Subject: [PATCH 6/7] let to const --- src/core/middleware/focus.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/middleware/focus.ts b/src/core/middleware/focus.ts index 8b81ec3f0..22a71ad06 100644 --- a/src/core/middleware/focus.ts +++ b/src/core/middleware/focus.ts @@ -12,7 +12,7 @@ export const focus = factory(({ middleware: { icache, cache, diffProperty, node, diffProperty('focus', (_: FocusProperties, next: FocusProperties) => { const result = next.focus && next.focus(); if (result) { - let current = icache.get('current') || 0; + const current = icache.get('current') || 0; icache.set('current', current + 1); } }); @@ -37,7 +37,7 @@ export const focus = factory(({ middleware: { icache, cache, diffProperty, node, return current !== previous; }, focus(): void { - let current = cache.get('current') || 0; + const current = cache.get('current') || 0; icache.set('current', current + 1); }, isFocused(key: string | number): boolean { From 8adfd2d344b73c7544cfe9bf3a8eef9f772fb611 Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 14 Jun 2019 17:15:56 +0100 Subject: [PATCH 7/7] Only invalidate if the previous active element is different to current active element --- src/core/middleware/focus.ts | 2 +- tests/core/unit/middleware/focus.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/middleware/focus.ts b/src/core/middleware/focus.ts index 22a71ad06..2680a74e2 100644 --- a/src/core/middleware/focus.ts +++ b/src/core/middleware/focus.ts @@ -19,7 +19,7 @@ export const focus = factory(({ middleware: { icache, cache, diffProperty, node, function onFocusChange() { const currentElement = cache.get('active-element'); const activeElement = global.document.activeElement; - if (nodeSet.has(currentElement) || nodeSet.has(activeElement)) { + if ((nodeSet.has(currentElement) || nodeSet.has(activeElement)) && currentElement !== activeElement) { invalidator(); } cache.set('active-element', activeElement); diff --git a/tests/core/unit/middleware/focus.ts b/tests/core/unit/middleware/focus.ts index 782fa0968..89f18af3b 100644 --- a/tests/core/unit/middleware/focus.ts +++ b/tests/core/unit/middleware/focus.ts @@ -121,8 +121,11 @@ describe('focus middleware', () => { }); const div = global.document.createElement('div'); const buttonOne = global.document.createElement('button'); + buttonOne.setAttribute('id', 'one'); const buttonTwo = global.document.createElement('button'); + buttonTwo.setAttribute('id', 'two'); const buttonThree = global.document.createElement('button'); + buttonThree.setAttribute('id', 'three'); div.appendChild(buttonOne); div.appendChild(buttonTwo); div.appendChild(buttonThree);