AngleSharp / AngleSharp.Css

:angel: Library to enable support for cascading stylesheets in AngleSharp.
https://anglesharp.github.io
MIT License
72 stars 34 forks source link

ComputeCurrentStyle should resolve CSS variables #62

Closed The-Nutty closed 9 months ago

The-Nutty commented 4 years ago

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 commented 4 years ago

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

FlorianRappl commented 4 years ago

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!

The-Nutty commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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 commented 1 year ago

+1 for resolving css vars

FlorianRappl commented 9 months ago

Landed in devel.