Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: use Element.computedStyleMap() #2143

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 9 additions & 18 deletions packages/qunit-dom/lib/assertions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import isVisible from './assertions/is-visible.js';
import isDisabled from './assertions/is-disabled.js';
import matchesSelector from './assertions/matches-selector.js';
import collapseWhitespace from './helpers/collapse-whitespace.js';
import getComputedStyle from './helpers/get-computed-style.js';
import {
type IDOMElementDescriptor,
resolveDOMElement,
Expand All @@ -29,10 +30,6 @@ export interface ExistsOptions {
count: number;
}

type CSSStyleDeclarationProperty = keyof CSSStyleDeclaration;

type ActualCSSStyleDeclaration = Partial<Record<CSSStyleDeclarationProperty, unknown>>;
Comment on lines -32 to -34
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these types provide much value as they are only used internally, with a type assertion..

I think these types are better represented as a plain record, eg; Record<string, string>


/**
* @namespace
*/
Expand Down Expand Up @@ -832,26 +829,20 @@ export default class DOMAssertions {
let element = this.findTargetElement();
if (!element) return this;

let computedStyle = window.getComputedStyle(element, selector);
let expectedProperties = Object.keys(expected) as CSSStyleDeclarationProperty[];
let computedStyle = getComputedStyle(element, selector);
let expectedProperties = Object.keys(expected);
Comment on lines +832 to +833
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗑️ the type assertion

if (expectedProperties.length <= 0) {
throw new TypeError(
`Missing style expectations. There must be at least one style property in the passed in expectation object.`
);
}

let result = expectedProperties.every(
property =>
(computedStyle.getPropertyValue(property.toString()) || computedStyle[property]) ===
expected[property]
property => computedStyle[property] === expected[property]
Comment on lines -844 to +841
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

camelCase and kebab-case properties are handled by getComputedStyle()

);
let actual: ActualCSSStyleDeclaration = {};
let actual: Record<string, string> = {};

expectedProperties.forEach(
property =>
(actual[property] =
computedStyle.getPropertyValue(property.toString()) || computedStyle[property])
);
expectedProperties.forEach(property => (actual[property] = computedStyle[property]));

if (!message) {
let normalizedSelector = selector ? selector.replace(/^:{0,2}/, '::') : '';
Expand Down Expand Up @@ -912,9 +903,9 @@ export default class DOMAssertions {
let element = this.findTargetElement();
if (!element) return this;

let computedStyle = window.getComputedStyle(element, selector);
let computedStyle = getComputedStyle(element, selector);

let expectedProperties = Object.keys(expected) as CSSStyleDeclarationProperty[];
let expectedProperties = Object.keys(expected);
if (expectedProperties.length <= 0) {
throw new TypeError(
`Missing style expectations. There must be at least one style property in the passed in expectation object.`
Expand All @@ -924,7 +915,7 @@ export default class DOMAssertions {
let result = expectedProperties.some(
property => computedStyle[property] !== expected[property]
);
let actual: ActualCSSStyleDeclaration = {};
let actual: Record<string, string> = {};

expectedProperties.forEach(property => (actual[property] = computedStyle[property]));

Expand Down
92 changes: 92 additions & 0 deletions packages/qunit-dom/lib/helpers/get-computed-style.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { describe, beforeEach, test, expect, vi } from 'vitest';

import getComputedStyle from './get-computed-style';

describe('getComputedStyle', () => {
beforeEach(() => {
vi.restoreAllMocks();
});

test('when computedStyleMap() is supported', () => {
Element.prototype.computedStyleMap = vi.fn().mockReturnValue(new Map([['width', '100px']]));

const element = document.createElement('div');
document.body.appendChild(element);

const computedStyle = getComputedStyle(element);
expect(computedStyle).toHaveProperty('width', '100px');
});

test('when computedStyleMap() is not supported', () => {
const element = document.createElement('div');
element.style.width = '100px';
document.body.appendChild(element);

const computedStyle = getComputedStyle(element);
expect(computedStyle).toHaveProperty('width', '100px');
});

test('kebab-case properties', () => {
const element = document.createElement('div');
element.style.textAlign = 'center';
document.body.appendChild(element);

const computedStyle = getComputedStyle(element);
expect(computedStyle).toHaveProperty('text-align', 'center');
});

test('camelCase properties', () => {
const element = document.createElement('div');
element.style.textAlign = 'center';
document.body.appendChild(element);

const computedStyle = getComputedStyle(element);
expect(computedStyle).toHaveProperty('textAlign', 'center');
});

test('iterating over StylePropertyMap properties', () => {
Element.prototype.computedStyleMap = vi.fn().mockReturnValue(
new Map([
['height', '200px'],
['width', '100px'],
])
);

const element = document.createElement('div');
document.body.appendChild(element);

const computedStyle = getComputedStyle(element);
expect(Object.keys(computedStyle)).toEqual(['height', 'width']);
});

test('iterating over CSSStyleDeclaration properties', () => {
const element = document.createElement('div');
element.style.height = '200px';
element.style.width = '100px';
document.body.appendChild(element);

const computedStyle = getComputedStyle(element);
expect(Object.keys(computedStyle)).toEqual(expect.arrayContaining(['0', '1', '2', '3']));
});

test('the existence of a StylePropertyMap property', () => {
const element = document.createElement('div');
element.style.textAlign = '200px';
document.body.appendChild(element);

const computedStyle = getComputedStyle(element);
expect(Reflect.has(computedStyle, 'text-align')).toBe(true);
});

test('the existence of a CSSStyleDeclaration property', () => {
Element.prototype.computedStyleMap = vi
.fn()
.mockReturnValue(new Map([['text-align', 'center']]));

const element = document.createElement('div');
document.body.appendChild(element);

const computedStyle = getComputedStyle(element);
expect(Reflect.has(computedStyle, 'text-align')).toBe(true);
});
});
57 changes: 57 additions & 0 deletions packages/qunit-dom/lib/helpers/get-computed-style.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
export default function getComputedStyle(element: Element, selector?: string | null) {
let computedStyleMap: StylePropertyMapReadOnly | undefined;

if (!selector && 'computedStyleMap' in element) {
computedStyleMap = element.computedStyleMap();
}
Comment on lines +4 to +6
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Element.computedStyleMap() doesn't work with with pseudo-elements, so it should fallback to window.getComputedStyle() in those cases


const computedStyle = window.getComputedStyle(element, selector);

return new Proxy<Record<string, string>>(
{},
{
get(_, property) {
let value;

if (typeof property === 'string' && computedStyleMap) {
value = computedStyleMap.get(property)?.toString();
}

return (
value ||
computedStyle.getPropertyValue(property.toString()) ||
Reflect.get(computedStyle, property)
);
Comment on lines +20 to +24
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintains the previous logic of

computedStyle.getPropertyValue(property.toString()) || computedStyle[property]

},

has(_, property) {
if (computedStyleMap && typeof property === 'string') {
return computedStyleMap.has(property);
}

return Reflect.has(computedStyle, property);
},

ownKeys(_) {
if (computedStyleMap) {
return Array.from(computedStyleMap.keys());
}

return Reflect.ownKeys(computedStyle);
},

getOwnPropertyDescriptor(_, property) {
if (computedStyleMap && typeof property == 'string') {
return {
writable: false,
configurable: true,
enumerable: true,
value: computedStyleMap.get(property),
};
}

return Reflect.getOwnPropertyDescriptor(computedStyle, property);
},
}
);
}
3 changes: 2 additions & 1 deletion packages/qunit-dom/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"devDependencies": {
"@arethetypeswrong/cli": "0.15.4",
"@types/qunit": "2.19.10",
"@typescript/lib-dom": "npm:@types/[email protected]",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"@typescript-eslint/eslint-plugin": "7.18.0",
"@typescript-eslint/parser": "7.18.0",
"@vitest/coverage-v8": "2.0.5",
Expand Down Expand Up @@ -71,4 +72,4 @@
"volta": {
"extends": "../../package.json"
}
}
}
7 changes: 7 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading