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

hasStyle does not work for CSS variables #1272

Closed
DingoEatingFuzz opened this issue Oct 10, 2021 · 4 comments
Closed

hasStyle does not work for CSS variables #1272

DingoEatingFuzz opened this issue Oct 10, 2021 · 4 comments

Comments

@DingoEatingFuzz
Copy link

The bug

Consider this element:

<div style="--my-var:123" />

The following assertion does not work:

assert.dom('div').hasStyle({ '--my-var': 123 });

Since hasStyle is implemented with getComputedStyle and getComputedStyle returns styles after computations are resolved, variable styles are not present on the CSSDeclaration object.

In order to get the variable styles from an element you must use element.style.getPropertyValue('--my-var').

Motivation to have this behavior

An emerging pattern now that CSS variables have broad adoption is to dynamically set variables in JS/HTML and then read the variables in static CSS.

For instance:

div {
  --my-var: 100; /* default value */
  height: var(--my-var); /* overridable from JS without hardcoding height */
  max-height: var(--my-var); /* implementation detail not the concern of JS */
}

Proposed solution

I think the API for hasStyle can remain unchanged. The keys for the expected object can be divided into variables (start with --) and declarations. Then the two sets can be iterated over using style.getPropertyValue and getComputedStyle respectively.

@monovertex
Copy link

I came upon this feature request while looking for the same functionality. If this is something wanted in the proposed form I can take a stab at implementing this and opening a PR.

@vladucu
Copy link

vladucu commented Mar 28, 2024

We would love to see support added for this.
An attempt to implement this already exist in #1976. Anything I can do to get this merged?

@BobrImperator
Copy link
Contributor

3.1.0 has been released https://www.npmjs.com/package/qunit-dom/v/3.1.0

See if it works for you

@BobrImperator
Copy link
Contributor

Closing this since we've released the 3.1.0 version, if the issue is still present, feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants