Skip to content

Commit

Permalink
Respond to review feedback with improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
amacdonald-google authored and uxder committed Jan 5, 2021
1 parent 6b734ac commit 1b06ca7
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 25 deletions.
2 changes: 1 addition & 1 deletion src/dom/dimensions-2d-dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
3 changes: 2 additions & 1 deletion src/dom/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
4 changes: 1 addition & 3 deletions src/dom/get-visible-y-from-root.ts
Original file line number Diff line number Diff line change
@@ -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';
}
Expand All @@ -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;
}
Expand Down
35 changes: 20 additions & 15 deletions src/dom/matrix-dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(<string>a);
this.b = parseFloat(<string>b);
this.c = parseFloat(<string>c);
this.d = parseFloat(<string>d);
this.tx = parseFloat(<string>tx);
this.ty = parseFloat(<string>ty);
this.a = a;
this.b = b;
this.c = c;
this.d = d;
this.tx = tx;
this.ty = ty;
}

getTranslateX(): number {
Expand Down
2 changes: 1 addition & 1 deletion src/map/array.spec.ts → src/map/array-map.spec.ts
Original file line number Diff line number Diff line change
@@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion src/map/array.ts → src/map/array-map.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/map/default.spec.ts → src/map/default-map.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DefaultMap } from './default';
import { DefaultMap } from './default-map';
import test from 'ava';

const testDefaultFns = [
Expand Down
7 changes: 5 additions & 2 deletions src/map/default.ts → src/map/default-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ export class DefaultMap<K, V> extends Map<K, V> {

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);
}
}

0 comments on commit 1b06ca7

Please sign in to comment.