From 10ef4a27fd2d4863afe6eca56d8958ed6558ef32 Mon Sep 17 00:00:00 2001 From: smhigley Date: Sun, 1 Jul 2018 23:26:25 -0700 Subject: [PATCH 1/3] streamline focus mixin --- src/mixins/Focus.ts | 49 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/src/mixins/Focus.ts b/src/mixins/Focus.ts index 8de262e2..1f2f4f47 100644 --- a/src/mixins/Focus.ts +++ b/src/mixins/Focus.ts @@ -1,14 +1,17 @@ import { Constructor } from './../interfaces'; import { WidgetBase } from './../WidgetBase'; +import { DNode, VNode, WNode } from '../interfaces'; +import { decorate, isWNode, isVNode } from '../d'; import { diffProperty } from './../decorators/diffProperty'; +import { afterRender } from './../decorators/afterRender'; export interface FocusProperties { focus?: (() => boolean); } export interface FocusMixin { - focus: () => void; - shouldFocus: () => boolean; + focus: (key: string) => void; + focusKey?: string | number; properties: FocusProperties; } @@ -23,23 +26,61 @@ function diffFocus(previousProperty: Function, newProperty: Function) { export function FocusMixin>>(Base: T): T & Constructor { abstract class Focus extends Base { public abstract properties: FocusProperties; + public focusKey: string | number; private _currentToken = 0; private _previousToken = 0; + private _currentFocusKey: string | number; + + private _shouldFocusChild() { + return this._currentFocusKey && this._shouldFocus(); + } + + private _shouldFocusSelf() { + return this.properties.focus && this.focusKey !== undefined; + } + @diffProperty('focus', diffFocus) protected isFocusedReaction() { this._currentToken++; } - public shouldFocus = () => { + @afterRender() + protected updateFocusProperties(result: DNode | DNode[]): DNode | DNode[] { + if (!this._shouldFocusChild() && !this._shouldFocusSelf()) { + return result; + } + + decorate(result, { + modifier: (node, breaker) => { + if (this._shouldFocusSelf() && node.properties.key === this.focusKey) { + node.properties = { ...node.properties, focus: this.properties.focus }; + } else if (node.properties.key === this._currentFocusKey) { + node.properties = { ...node.properties, focus: () => true }; + } + breaker(); + }, + predicate: (node: DNode): node is VNode | WNode => { + if (!isVNode(node) && !isWNode(node)) { + return false; + } + const { key } = node.properties; + return key === this._currentFocusKey || (!!this.focusKey && key === this.focusKey); + } + }); + return result; + } + + private _shouldFocus = () => { const result = this._currentToken !== this._previousToken; this._previousToken = this._currentToken; return result; }; - public focus() { + public focus(key: string | number) { + this._currentFocusKey = key; this._currentToken++; this.invalidate(); } From 1dd93a99e8521f437b091fb9b741bd67dccd79fe Mon Sep 17 00:00:00 2001 From: smhigley Date: Fri, 6 Jul 2018 21:31:44 -0700 Subject: [PATCH 2/3] added comments and tests --- src/mixins/Focus.ts | 38 ++++++-- tests/unit/mixins/Focus.ts | 193 +++++++++++++++++++++++++++++++------ 2 files changed, 194 insertions(+), 37 deletions(-) diff --git a/src/mixins/Focus.ts b/src/mixins/Focus.ts index 1f2f4f47..760ec293 100644 --- a/src/mixins/Focus.ts +++ b/src/mixins/Focus.ts @@ -1,24 +1,39 @@ import { Constructor } from './../interfaces'; import { WidgetBase } from './../WidgetBase'; -import { DNode, VNode, WNode } from '../interfaces'; +import { DNode, VNode, WNode, NodeOperationPredicate } from '../interfaces'; import { decorate, isWNode, isVNode } from '../d'; import { diffProperty } from './../decorators/diffProperty'; import { afterRender } from './../decorators/afterRender'; export interface FocusProperties { - focus?: (() => boolean); + /** + * The focus property allows a widget to be focused by a parent. + * It must be used in conjunction with focusKey, or have a custom implementation within the widget + */ + focus?: boolean | NodeOperationPredicate; } export interface FocusMixin { + /** + * The focus method marks a specific node for decoration with focus: () => true; + */ focus: (key: string) => void; + /** + * The focusKey property is used with the focus widget property to allow a widget to be focused by a parent. + * If present, a truthy FocusProperties.focus will decorate this node with focus: () => true + */ focusKey?: string | number; properties: FocusProperties; } function diffFocus(previousProperty: Function, newProperty: Function) { - const result = newProperty && newProperty(); + let changed = newProperty !== previousProperty; + if (typeof newProperty === 'function') { + changed = newProperty(); + } + return { - changed: result, + changed, value: newProperty }; } @@ -39,14 +54,14 @@ export function FocusMixin>>(B } private _shouldFocusSelf() { - return this.properties.focus && this.focusKey !== undefined; + let { focus } = this.properties; + if (typeof focus === 'function') { + focus = focus(); + } + return focus && this.focusKey !== undefined; } @diffProperty('focus', diffFocus) - protected isFocusedReaction() { - this._currentToken++; - } - @afterRender() protected updateFocusProperties(result: DNode | DNode[]): DNode | DNode[] { if (!this._shouldFocusChild() && !this._shouldFocusSelf()) { @@ -79,6 +94,11 @@ export function FocusMixin>>(B return result; }; + /** + * Will mark a node for decoration with the focus property and trigger a render + * + * @param key The key to call focus on + */ public focus(key: string | number) { this._currentFocusKey = key; this._currentToken++; diff --git a/tests/unit/mixins/Focus.ts b/tests/unit/mixins/Focus.ts index 4c121833..9c9231ba 100644 --- a/tests/unit/mixins/Focus.ts +++ b/tests/unit/mixins/Focus.ts @@ -1,39 +1,176 @@ const { describe, it } = intern.getInterface('bdd'); const { assert } = intern.getPlugin('chai'); +import { stub } from 'sinon'; +import { v, w } from '../../../src/d'; import { WidgetBase } from '../../../src/WidgetBase'; import Focus from '../../../src/mixins/Focus'; -class Foo extends Focus(WidgetBase) {} - describe('Focus Mixin', () => { - it('should allow once focus when focus property returns true', () => { - const widget = new Foo(); - widget.__setProperties__({ focus: () => true }); - assert.isTrue(widget.shouldFocus()); - assert.isFalse(widget.shouldFocus()); - widget.focus(); - assert.isTrue(widget.shouldFocus()); - assert.isFalse(widget.shouldFocus()); - }); + describe('this.focus()', () => { + it('should allow focus once per this.focus()', () => { + class Focusable extends Focus(WidgetBase) { + render() { + return v('button', { key: 'button' }); + } + } + + const widget = new Focusable(); + widget.focus('button'); + + let result = widget.__render__(); + assert.isFunction((result as any).properties!.focus); + + widget.invalidate(); + result = widget.__render__(); + assert.isUndefined((result as any).properties!.focus); + }); + + it('should not focus without a key', () => { + class Focusable extends Focus(WidgetBase) { + render() { + return v('button', {}); + } + } + + const widget = new Focusable(); + widget.focus('button'); + + let result = widget.__render__(); + assert.isUndefined((result as any).properties!.focus); + }); + + it('should not focus incorrect key', () => { + class Focusable extends Focus(WidgetBase) { + render() { + return v('button', { key: 'button' }); + } + } + + const widget = new Focusable(); + widget.focus('btn'); + + let result = widget.__render__(); + assert.isUndefined((result as any).properties!.focus); + }); + + it('can call focus twice', () => { + class Focusable extends Focus(WidgetBase) { + render() { + return v('button', { key: 'button' }); + } + } + + const widget = new Focusable(); + widget.focus('button'); + + let result = widget.__render__(); + assert.isFunction((result as any).properties!.focus); + + widget.invalidate(); + widget.focus('button'); + result = widget.__render__(); + assert.isFunction((result as any).properties!.focus); + }); + + it('should set focus on child widgets', () => { + class Child extends Focus(WidgetBase) {} + + class Parent extends Focus(WidgetBase) { + render() { + return w(Child, { key: 'child' }); + } + } - it('should not focus when focus property returns false', () => { - const widget = new Foo(); - widget.__setProperties__({ focus: () => false }); - assert.isFalse(widget.shouldFocus()); - widget.focus(); - assert.isTrue(widget.shouldFocus()); - assert.isFalse(widget.shouldFocus()); - widget.__setProperties__({ focus: () => true }); - assert.isTrue(widget.shouldFocus()); - assert.isFalse(widget.shouldFocus()); + const widget = new Parent(); + widget.focus('child'); + let result = widget.__render__(); + assert.strictEqual((result as any).properties!.key, 'child'); + assert.isFunction((result as any).properties!.focus); + }); }); - it('should not focus when is not passed', () => { - const widget = new Foo(); - widget.__setProperties__({}); - assert.isFalse(widget.shouldFocus()); - widget.focus(); - assert.isTrue(widget.shouldFocus()); - assert.isFalse(widget.shouldFocus()); + describe('properties.focus', () => { + it('should pass focus property to child', () => { + class Focusable extends Focus(WidgetBase) { + focusKey = 'button'; + render() { + return v('button', { key: 'button' }); + } + } + + const focusStub = stub().returns(true); + const widget = new Focusable(); + widget.__setProperties__({ focus: focusStub }); + + let result = widget.__render__(); + assert.isTrue((result as any).properties!.focus()); + }); + + it('should not pass focus property with no focusKey', () => { + class Focusable extends Focus(WidgetBase) { + render() { + return v('button', { key: 'button' }); + } + } + + const widget = new Focusable(); + widget.__setProperties__({ focus: true }); + + let result = widget.__render__(); + assert.isUndefined((result as any).properties!.focus); + }); + + it('should not pass focus property with incorrect focusKey', () => { + class Focusable extends Focus(WidgetBase) { + focusKey = 'btn'; + render() { + return v('button', { key: 'button' }); + } + } + + const widget = new Focusable(); + widget.__setProperties__({ focus: true }); + + let result = widget.__render__(); + assert.isUndefined((result as any).properties!.focus); + }); + + it('should always pass focus property to child', () => { + class Focusable extends Focus(WidgetBase) { + focusKey = 'button'; + render() { + return v('button', { key: 'button' }); + } + } + + const widget = new Focusable(); + widget.__setProperties__({ focus: true }); + + let result = widget.__render__(); + assert.isTrue((result as any).properties!.focus); + + widget.invalidate(); + result = widget.__render__(); + assert.isTrue((result as any).properties!.focus); + }); + + it('should diff focus function', () => { + class Focusable extends Focus(WidgetBase) { + focusKey = 'button'; + render() { + return v('button', { key: 'button' }); + } + } + + const widget = new Focusable(); + widget.__setProperties__({ focus: () => true }); + + let result = widget.__render__(); + assert.isTrue((result as any).properties!.focus()); + + widget.__setProperties__({ focus: () => false }); + result = widget.__render__(); + assert.isUndefined((result as any).properties!.focus); + }); }); }); From b05d695c3e4ab2c03b8a8546466eed340e7c73fd Mon Sep 17 00:00:00 2001 From: smhigley Date: Fri, 6 Jul 2018 22:15:17 -0700 Subject: [PATCH 3/3] update readme --- README.md | 41 +++++++++++++---------------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index efc68864..5af3af33 100644 --- a/README.md +++ b/README.md @@ -649,9 +649,9 @@ v('input', { type: 'text', focus: () => true) }) This primitive is a base that enables further abstractions to be built to handle more complex behaviors. One of these is handling focus across the boundaries of encapsulated widgets. The `FocusMixin` should be used by widgets to provide `focus` to their children or to accept `focus` from a parent widget. -The `FocusMixin` adds `focus` and `shouldFocus` to a widget's API. `shouldFocus` checks if the widget is in a state to perform a focus action and will only return `true` once until the widget's `focus` method has been called again. This `shouldFocus` method is designed to be passed to child widgets or nodes are the value of their `focus` property. +The `FocusMixin` adds `focus` to a widget's API. When called with a key, `focus` decorates the corresponding child node with `focus: () => true` for one render. -When `shouldFocus` is passed to a widget it will be called as the properties are set on the child widget, meaning that any other usages of the parent's `shouldFocus` method will result in a return value of `false`. +The `FocusMixin` can also be used to handle receiving a `focus` property from a parent widget. If `focusKey` is defined on the widget class, the `FocusMixin` will pass on `widget.properties.focus` to the corresponding child node. An example usage controlling focus across child VNodes (DOM) and WNodes (widgets): @@ -661,9 +661,10 @@ An example usage controlling focus across child VNodes (DOM) and WNodes (widgets } class FocusInputChild extends Focus(WidgetBase) { + // focusKey must be defined, or nothing will happen if the widget is passed a truthy focus property + focusKey = 'input'; protected render() { - // the child widget's `this.shouldFocus` is passed directly to the input nodes focus property - return v('input', { onfocus: this.properties.onFocus, focus: this.shouldFocus }); + return v('input', { key: 'input', onfocus: this.properties.onFocus }); } } @@ -676,59 +677,43 @@ An example usage controlling focus across child VNodes (DOM) and WNodes (widgets } private _previous() { - if (this._focusedInputKey === 0) { + this._focusedInputKey--; + if (this._focusedInputKey < 0) { this._focusedInputKey = 4; - } else { - this._focusedInputKey--; } - // calling focus resets the widget so that `this.shouldFocus` - // will return true on its next use - this.focus(); + + // call this.focus with the updated focus key + this.focus(this._focusedInputKey); } private _next() { - if (this._focusedInputKey === 4) { - this._focusedInputKey = 0; - } else { - this._focusedInputKey++; - } - // calling focus resets the widget so that `this.shouldFocus` - // will return true on its next use - this.focus(); + this._focusedInputKey = (this._focusedInputKey + 1) % 5; + // call this.focus with the updated focus key + this.focus(this._focusedInputKey); } protected render() { return v('div', [ v('button', { onclick: this._previous }, ['Previous']), v('button', { onclick: this._next }, ['Next']), - // `this.shouldFocus` is passed to the child that requires focus based on - // some widget logic. If the child is a widget it can then deal with that - // however is necessary. The widget may also have internal logic and pass - // its own `this.shouldFocus` down further or could apply directly to a - // VNode child. w(FocusInputChild, { key: 0, - focus: this._focusedInputKey === 0 ? this.shouldFocus : undefined, onFocus: () => this._onFocus(0) }), w(FocusInputChild, { key: 1, - focus: this._focusedInputKey === 1 ? this.shouldFocus : undefined, onFocus: () => this._onFocus(1) }), w(FocusInputChild, { key: 2, - focus: this._focusedInputKey === 2 ? this.shouldFocus : undefined, onFocus: () => this._onFocus(2) }), w(FocusInputChild, { key: 3, - focus: this._focusedInputKey === 3 ? this.shouldFocus : undefined, onFocus: () => this._onFocus(3) }), v('input', { key: 4, - focus: this._focusedInputKey === 4 ? this.shouldFocus : undefined, onfocus: () => this._onFocus(4) }) ]);