Skip to content

ComputeCurrentStyle should resolve CSS variables #62

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.

Sign up for GitHub

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

Jump to bottom
Closed
The-Nutty opened this issue Sep 17, 2020 · 9 comments
Closed

ComputeCurrentStyle should resolve CSS variables #62

The-Nutty opened this issue Sep 17, 2020 · 9 comments
Labels
api cssom enhancement help-wanted style-computation
Milestone
v1.0

Comments

@The-Nutty
Copy link
Contributor

The-Nutty commented Sep 17, 2020 •
edited
Loading

New Feature Proposal

Description

When using IElement.ComputeCurrentStyle i would expect CSS variables to be resolved for example when parsing the document:

<!DOCTYPE html>
<html>
<head>
    <style>
        :root {
            --color: #FFFFFF;
        }

        p {
            color: var(--color);
        }
    </style>
</head>
<body>
    <p>This is a test</p>
</body>
</html>

I would expect doc.GetDescendantsAndSelf().OfType<IHtmlParagraphElement>().First().ComputeCurrentStyle()["color"] == "#FFFFFF" for example. But instead its rgb(var(--color)). However i would understand if ComputeCurrentStyle should not resolve CSS variables if thats the case i think it would be useful to have an extention method or utility that can do this.

Is this something you are interested in adding or if you pointed me in the place you this method should belong i could have a crack at submitting a PR.
@The-Nutty
Copy link
Contributor Author

I have just check chromes behavure when using window.getComputedStyle and that appears to resolve CSS variable's before returning.

@FlorianRappl
Copy link
Contributor

Thanks for checking @The-Nutty !

Yes, indeed it would be wonderful if you could look into this (to update the behavior of ComputeCurrentStyle ). Would be very much appreciated!

@FlorianRappl FlorianRappl added this to the v1.0 milestone Sep 18, 2020
@The-Nutty
Copy link
Contributor Author

So in the document above in chrome document.styleSheets[0].cssRules[1].style["color"] == "var(--color)". Which suggests it does not want to be done in the CssStyleDeclaration#SetDeclarations/UpdateDeclarations.

Could add an extension method on CssStyleDeclaration that iterates through all decelerations and resolves var() expressions. This would require an extra method on CssStyleDeclaration like void ReplaceCssProperty(ICssProperty) that will just always replace it by name without thinking.
As for actually resolving var expressions it needs to be able to resolve things like var(--style1, black) and var(--style1, var(--style2, black)) i guess the easy way to do this is recursively with a regex like var\((.+)(?:,(.+))?\) however a better idea might be to use an actual parser. Do you have a preference, does this sound like the correct approach?

@The-Nutty
Copy link
Contributor Author

@FlorianRappl I have had another look at this and think it should be done during parsing at the same time we do things like calc as calc(var(--number-var) * 1px) is valid for example. However with the way converters are currently done i think the only way todo it is to edit each value converter parser adding support for it? Or edit the implementation of DefaultDeclarationFactory#Create (and CreateDefault?) to resolve var() expressions before going to the converters. However im not sure how possible that is with the current parsing code?

@FlorianRappl
Copy link
Contributor

FlorianRappl commented Oct 7, 2020 •
edited
Loading

Hm evaluating CSS variables during parsing does not sound right to me. The value of the CSS variable can only be determined at runtime when used in CSSOM, as the cascade is used to set the context-dependent value.

Am I on the wrong track here? cc @The-Nutty

@The-Nutty
Copy link
Contributor Author

No i do agree with you, im just trying to fit it into the current architecture, and it needs to happen before calc stuff, and as i understand it thats done during parsing? Im not 100% sure where this code should live, im open to any suggestions or pointers to the right place in code.

@The-Nutty
Copy link
Contributor Author

No i do agree with you, im just trying to fit it into the current architecture, and it needs to happen before calc stuff, and as i understand it thats done during parsing? Im not 100% sure where this code should live, im open to any suggestions or pointers to the right place in code.

@FlorianRappl Have you had a chance to take a look at this?

@jmalatia
Copy link

+1 for resolving css vars

FlorianRappl added a commit that referenced this issue Jan 18, 2024
@FlorianRappl
Implemented CSS variable resolution #62
8295892
@FlorianRappl
Copy link
Contributor

Landed in devel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api cssom enhancement help-wanted style-computation
Projects
None yet
Milestone
v1.0
Development

No branches or pull requests
3 participants
@FlorianRappl @jmalatia @The-Nutty