From 1b06ca7d4f530e6e352c170e24cfc6d9918c222e Mon Sep 17 00:00:00 2001 From: Angus MacDonald Date: Sat, 19 Dec 2020 12:41:43 -0600 Subject: [PATCH] Respond to review feedback with improvements --- src/dom/dimensions-2d-dom.ts | 2 +- src/dom/dom.ts | 3 +- src/dom/get-visible-y-from-root.ts | 4 +-- src/dom/matrix-dom.ts | 35 +++++++++++-------- src/map/{array.spec.ts => array-map.spec.ts} | 2 +- src/map/{array.ts => array-map.ts} | 2 +- .../{default.spec.ts => default-map.spec.ts} | 2 +- src/map/{default.ts => default-map.ts} | 7 ++-- 8 files changed, 32 insertions(+), 25 deletions(-) rename src/map/{array.spec.ts => array-map.spec.ts} (91%) rename src/map/{array.ts => array-map.ts} (87%) rename src/map/{default.spec.ts => default-map.spec.ts} (96%) rename src/map/{default.ts => default-map.ts} (85%) diff --git a/src/dom/dimensions-2d-dom.ts b/src/dom/dimensions-2d-dom.ts index e747f3905d..40be0c5d59 100644 --- a/src/dom/dimensions-2d-dom.ts +++ b/src/dom/dimensions-2d-dom.ts @@ -16,6 +16,6 @@ export class Dimensions2dDom { } getHypotenuseLength() { - return Math.sqrt(Math.pow(this.width, 2) + Math.pow(this.height, 2)); + return Math.sqrt(this.width * this.width + this.height * this.height); } } diff --git a/src/dom/dom.ts b/src/dom/dom.ts index 40a61327e1..02b8e1bd86 100644 --- a/src/dom/dom.ts +++ b/src/dom/dom.ts @@ -847,7 +847,8 @@ export class dom { * * ``` */ - static getStyle(el: Element): CSSStyleDeclaration {return el['currentStyle'] || window.getComputedStyle(el); + static getStyle(el: Element): CSSStyleDeclaration { + return el['currentStyle'] || window.getComputedStyle(el); } /** diff --git a/src/dom/get-visible-y-from-root.ts b/src/dom/get-visible-y-from-root.ts index e11136db1f..096696fdb3 100644 --- a/src/dom/get-visible-y-from-root.ts +++ b/src/dom/get-visible-y-from-root.ts @@ -1,8 +1,6 @@ import { dom } from '../'; import { domVectorf } from './dom-vectorf'; -const ignoredPositions = new Set(['fixed', 'absolute']); - function isFixed(element: HTMLElement) { return dom.getStyle(element).position === 'fixed'; } @@ -16,7 +14,7 @@ function getVisibleDistanceFromRoot( element: HTMLElement, getOffsetFn: (e: HTMLElement) => number ): number { - // Short circuit for fixed elements + // Short circuit for fixed elements. if (isFixed(element)) { return element.offsetTop; } diff --git a/src/dom/matrix-dom.ts b/src/dom/matrix-dom.ts index 20956259a3..debfb54149 100644 --- a/src/dom/matrix-dom.ts +++ b/src/dom/matrix-dom.ts @@ -5,14 +5,19 @@ */ import { dom } from './dom'; -type Numeric = number|string; // Number that could be represented as string - export class MatrixDom { static parseFromString(str: string): MatrixDom { if (str === 'none' || !str.length) { return new MatrixDom(); } - return new MatrixDom(...str.slice('matrix('.length, -1).split(',')); + const values = + str.slice('matrix('.length, -1) + .split(',') + .map((value) => parseFloat(value)); + if (values.length !== 6 || values.some((value) => isNaN((value)))) { + throw new Error('Invalid matrix passed to MatrixDom.parseFromString'); + } + return new MatrixDom(...values); } static fromElementTransform(element: Element): MatrixDom { @@ -30,19 +35,19 @@ export class MatrixDom { // Use numeric to allow strings or numbers constructor( - a: Numeric = 1, - b: Numeric = 0, - c: Numeric = 0, - d: Numeric = 1, - tx: Numeric = 0, - ty: Numeric = 0 + a: number = 1, + b: number = 0, + c: number = 0, + d: number = 1, + tx: number = 0, + ty: number = 0 ) { - this.a = parseFloat(a); - this.b = parseFloat(b); - this.c = parseFloat(c); - this.d = parseFloat(d); - this.tx = parseFloat(tx); - this.ty = parseFloat(ty); + this.a = a; + this.b = b; + this.c = c; + this.d = d; + this.tx = tx; + this.ty = ty; } getTranslateX(): number { diff --git a/src/map/array.spec.ts b/src/map/array-map.spec.ts similarity index 91% rename from src/map/array.spec.ts rename to src/map/array-map.spec.ts index 8688c52a27..fe93ff198f 100644 --- a/src/map/array.spec.ts +++ b/src/map/array-map.spec.ts @@ -1,4 +1,4 @@ -import { ArrayMap } from './array'; +import { ArrayMap } from './array-map'; import test from 'ava'; test('ArrayMap should always return an array', async t => { diff --git a/src/map/array.ts b/src/map/array-map.ts similarity index 87% rename from src/map/array.ts rename to src/map/array-map.ts index 773cd6e455..8fbab41b04 100644 --- a/src/map/array.ts +++ b/src/map/array-map.ts @@ -1,4 +1,4 @@ -import { DefaultMap } from './default'; +import { DefaultMap } from './default-map'; /** * A map where the values in the map are all expected to be arrays. diff --git a/src/map/default.spec.ts b/src/map/default-map.spec.ts similarity index 96% rename from src/map/default.spec.ts rename to src/map/default-map.spec.ts index 95862c8899..a7df63405b 100644 --- a/src/map/default.spec.ts +++ b/src/map/default-map.spec.ts @@ -1,4 +1,4 @@ -import { DefaultMap } from './default'; +import { DefaultMap } from './default-map'; import test from 'ava'; const testDefaultFns = [ diff --git a/src/map/default.ts b/src/map/default-map.ts similarity index 85% rename from src/map/default.ts rename to src/map/default-map.ts index 767025bb81..547f109916 100644 --- a/src/map/default.ts +++ b/src/map/default-map.ts @@ -30,8 +30,11 @@ export class DefaultMap extends Map { get(key: K): V { if (!this.has(key)) { - this.set(key, this.defaultFunction(key)); + const defaultValue = this.defaultFunction(key); + this.set(key, defaultValue); + return defaultValue; + } else { + return super.get(key); } - return super.get(key); } }