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

Bug: CSS Custom Properties can't be set #177

Open
hpate-omicron opened this issue Sep 4, 2018 · 32 comments
Open

Bug: CSS Custom Properties can't be set #177

hpate-omicron opened this issue Sep 4, 2018 · 32 comments

Comments

@hpate-omicron
Copy link

hpate-omicron commented Sep 4, 2018

CSS Variables cannot be used from Elm at the moment, style "--myvar" "value" is not applied to DOM.

Elm version: 0.19
Browsers: All

Here are some SSCEs, if CSS variables were applied you would see a 200x200 blue box in this Ellie:
https://ellie-app.com/3fpSpnv8nyNa1

Instead, the box is empty
image

Vanilla HTML + CSS example https://codepen.io/hpate-omicron/pen/BOZzgZ
image

caniuse data: https://caniuse.com/#feat=css-variables
W3 spec: https://www.w3.org/TR/css-variables/

@brasilikum
Copy link

Possible duplicate:
#129

@hpate-omicron
Copy link
Author

To address some of the notes on the previous issue:

I'm not sure the exact purpose of CSS variables when you are working in a language that has variables. It seems like this is a 2nd way to do things we can already do differently, except now without any compiler help for types and typos.

I'm not against doing this particularly, but so far I have not seen any cases that definitively show that this is a good thing to have. I prefer to use GitHub for bugs, not feature requests, so I will close.

The main benefit is that the variables can be referenced from other CSS, for instance, I have some code to allow me to make boxes at different aspect ratios. Another use is in theming components.

@supports (--custom: property) {
  [style*="--aspect-ratio"]::before {
    content: "";
    float: left;
    height: 0;
    width: 1px;
    margin-left: -1px;
    padding-bottom: calc(100% / var(--aspect-ratio));
  }
  [style*="--aspect-ratio"]::after {
    content: "";
    display: table;
    clear: both;
  }
}

To the second point, I believe this to be a bug because it doesn't not support the spec, a feature request would be to add something not supported in the CSS spec.

@hpate-omicron
Copy link
Author

Turns out there is already a PR that fixes this as well - elm/virtual-dom#127

@brasilikum
Copy link

@hpate-omicron So do you think you can close this as discussion is going on elsewhere anyways?

@hpate-omicron
Copy link
Author

This one should remain open, the previous issue was marked as a feature request and closed, and the pull request hasn't had any activity on it since it was opened, so this is still an open bug.

@Lattyware
Copy link

I'd like to note this is a really important bug to fix for my uses.

One of the core benefits of CSS is that style can be swapped out without changing the content - this means users can do things like having custom style-sheets. If you force the use of elm variables - that no longer works - the style is now embedded in the application and can't be changed in the same way.

If this feature is excluded, it basically means that Elm is dictating the use of inline styles, otherwise you have to refactor styles into elm code whenever you discover you need a variable. You then end up with duplication in your elm and CSS (e.g: if you have a media query that would interact with that variable, you now have to match that query with elm code to do the same thing, and keep it in sync).

@harrysarson
Copy link

harrysarson commented Oct 31, 2018

You can make a port and call out to it when custom variables need updating. That is how I am working around the bug currently.

In js

  app.ports.setCssProp.subscribe(([selector, prop, value]) => {
    for (const $el of document.querySelectorAll(selector)) {
      $el.style.setProperty(prop, value);
    }
  });

In elm

update : Msg -> Model -> ( Model, Cmd Msg )
update msg model =
    case msg of
        ChangeStage change ->
            let
                newStage =
                    max 0 <| min Config.maxStage (model.stage + change)
            in
            { model | stage = newStage }
                |> Cmd.Extra.with
                    (setCssProp ( ".display-panel", "--show-stage", String.fromInt newStage ))
		...
					

@berenddeboer
Copy link

Great workaround, but flabbergasted elm won't fix this.

@Lattyware
Copy link

Lattyware commented Aug 8, 2019

As noted in Lattyware/elm-fontawesome-generator#4 this is now a blocking issue for elm-fontawesome.

Custom properties can't simply be replaced with Elm code as suggested elsewhere. Given this seems like a small change that is already implemented, the cost of supporting this seems very low, and as far as I can see, is being held up based on misunderstanding more than anything else.

Furthermore, this is only going to get more important: future tools are being built on top of custom properties, and those will also break without them.

@ChristophP
Copy link

Yes, there are tons of libraries that are built on this the standard of CSS custom properties, such as this one. Each component can be customized with custom properties.
https://shoelace.style/components/image-comparer?id=css-custom-properties

There are even new specs building on top of it such as the @property keyword in CSS.
Example: https://css-tricks.com/using-property-for-css-custom-properties/
Spec: https://www.w3.org/TR/css-properties-values-api-1/#example-2

@z5h
Copy link

z5h commented Mar 15, 2021

Turns out there is already a PR that fixes this as well - elm/virtual-dom#127

Seeing as there is a fix, can we apply some "persuasion" to get this applied? Perhaps a writeup on Discourse that would help people understand the problem?

@Lattyware
Copy link

Lattyware commented Mar 16, 2021

Turns out there is already a PR that fixes this as well - elm/virtual-dom#127

Seeing as there is a fix, can we apply some "persuasion" to get this applied? Perhaps a writeup on Discourse that would help people understand the problem?

From April last year: https://discourse.elm-lang.org/t/css-custom-properties/5554

Radio silence after asking for the example. I lobbied for this one as hard as I could, but just no response. Evan seems to have decided that the language is frozen for now, so I wouldn't bother engaging until after things are moving again.

@ChristophP
Copy link

I remember back then that @Lattyware did a great job explaining why CSS custom properties should be supported and had lots of details in his response in discourse, which sadly never got any reactions from Evan.

However, I think the whole questioning whether it makes sense to support because elm already has variables is a bit like saying: Why should we support web components since you can already make components with Elm/React/Vue/Angular? The fact is that CSS Custom properties are a web standard that is seeing increasing usage and enables animating thins which weren't possible before in many areas including libs that Elm users may rely on.

So I think a better question than Should Elm support this? would be Will any priority be assigned to this given that it's an easy fix and it's impacting many users?

@harrysarson
Copy link

harrysarson commented Mar 17, 2021

Are the any insurmountable issues with patching this in the compiled js?

elm make ... --output elm-needs-patching.js
sed s/domNodeStyle[key] = styles[key]/domNodeStyle.setProperty(key, styles[key])/ elm-needs-patching.js > elm.js
# minify etc

otherwise that should unblock this for you

@ChristophP
Copy link

ChristophP commented Mar 17, 2021

@harrysarson I'm using a css class which sets whatever property needs to be set.

.color-red {
  color: var(--my-red); 
}

So I have a workaround for my use cases. I just see how much CSS vars are being used in all of the frontend world and I think we moved past the point of pondering whether this should be supported or not. To help the situation I think this issue should be addressed for Elm or if it's not an issue of whether this should be adopted it should be communicated that this simply is not a priority for the foreseeable future. I think that kind of communication would be fair to respect the efforts of @Lattyware and others who have tried to provide the information but never got a response.

@SuPythony
Copy link

Following on @harrysarson 's suggestion, I have created a batch script -

elm make %1 --optimize --output=unpatched.js
powershell -Command "(gc unpatched.js) -replace 'domNodeStyle\[key\] = styles\[key\]', 'domNodeStyle.setProperty(key, styles[key])' | Out-File -encoding ASCII main.js"

It can be used like this - compile.bat src/Main.elm

@SuPythony
Copy link

I am new to Elm and I can't understand why the pull request is not being merged. The solution for this problem is just to change that one line!

@ChristophP
Copy link

@SuPythony this is where the last conversation went silent https://discourse.elm-lang.org/t/css-custom-properties/5554.
There seemed to be concerns about performance implications of making the change (elm is usually benchmarked pretty well for performance) as well as seemingly concerns about whether CSS vars is in conflict with other kinsd of module systems.
IMO the web has moved on to fully embrace CSS custom properties and not supporting it at this point is a bummer for Elm users who want to use CSS vars or have to use them because of dependencies or component libs that rely on them. I wish had support for it without workarounds.

@SuPythony
Copy link

@ChristophP So does making this change make it slow? Also is there any other way to do it instead of just setting the style directly in elm? I was working with a stylesheet that was already made and wanted to change the css variables reactively using the model.

@harrysarson
Copy link

harrysarson commented Feb 21, 2022

See my PR elm/virtual-dom#127, there are back compat concerns so it cannot be merged as is. I don't think it would be impossible to resolve those though if someone wanted to try and drive this forward. Obviously involvement from maintainers is also needed for merge and this repo isn't under active dev at this point in time.

@SuPythony
Copy link

@harrysarson So for now, will changing the generated js like you said cause any problems or will it be fine?

@ChristophP
Copy link

@harrysarson ah I see, didn't know about those camel-case/kebap-case spelling issues. Yeah, that could lead to code breaking and would thus warrant a new major version of elm/html I guess.
So handling this would require some more time and attention, it's just a bit frustrating that solving this issue is so low on the priority list since quite a large number of people are bugged by this.

@Lattyware
Copy link

Would having a distinct customProperty function solve this? It could require/add the -- and therefore only work for custom properties, and do the right thing, thus avoiding performance issues and breaking compatibility.

It does make these things more distinct, but I'm not sure that's really a problem?

@SuPythony
Copy link

@Lattyware That's a great idea!

@ChristophP
Copy link

ChristophP commented Feb 22, 2022

@Lattyware I think that would be pretty nice and it be possible to add support without breaking changes. But I wonder if how well that fits into Elm's API design philosophy of the elm/html package, where everything basically mirrors HTML. One downside could be that one has to know about customProperty and could be suprised if style does not work as one would expect.
Might make the design a little less coherent, wouldn't bother me personally though.

@dboitnot
Copy link

This got me too. I was able to work around it by using Html.Attribute.attribute:

    H.span
        [ A.attribute "style" ("font-variation-settings:" ++ fontVariationSettings attrs)
        , A.class "material-symbols-outlined"
        ]
        [ H.text name ]

@ChristophP
Copy link

Yeah, that works. One downside of this this way of adding Styles is that it doesn't stack, i.e. calling it multiple times will override Styles instead of combining them.

@arowM
Copy link

arowM commented Jan 16, 2023

elm-mixin is another work around without the downside.
The Mixin.style function can handle multiple style attributes well.

@dwayne
Copy link

dwayne commented Apr 5, 2023

In elm-2048 I used the following snippet,

H.node "style"
    []
    [ H.text <| ":root { --grid-tile-move-duration: " ++ String.fromFloat moveDuration ++ "ms;" ++ " }"
    ]

re: https://github.com/dwayne/elm-2048/blob/607a1219024d8422263ed3bec5bb1b4bdc040035/src/App/View/Grid.elm#L115-L118

to set the CSS custom property --grid-tile-move-duration.

re: https://github.com/dwayne/elm-2048/blob/607a1219024d8422263ed3bec5bb1b4bdc040035/sass/_config.scss#L20

@mauroc8
Copy link

mauroc8 commented May 22, 2023

I have the same issue but a different use-case for CSS variables. I'll add here an example of what I would want to do hoping it helps:

.hover-background-color:hover {
  background-color: var(--hover-background-color);
}
hoverBackgroundColor : Color.Color -> List (Html.Attribute msg)
hoverBackgroundColor color =
  [ Html.Attributes.class "hover-background-color"
  , Html.Attributes.style "--hover-background-color" (Color.toCssString color)
  ]

I think CSS variables are really cool. We get to use CSS pseudo-classes with vainilla elm-html (without elm-css).

(The workaround, as mentioned, is to use elm-mixin)

@dwayne
Copy link

dwayne commented May 23, 2023

I've collected some notes on all the acceptable workarounds I've found.

@pete-murphy
Copy link

pete-murphy commented Nov 16, 2024

The workarounds I've seen (using attribute "style" or node "style", such as in elm-mixin) are not compatible with Content Security Policy without resorting to style-src 'unsafe-inline'.

While <style>...</style> and setAttribute("style", ..) are blocked by CSP, style[prop] = value and style.setProperty(prop, value) are not blocked.

The proposed change to use style.setProperty would allow for CSP compliant usage of custom properties.

An extremely limited workaround (if already using Tailwind) is to use Tailwind arbitrary value syntax class "[--custom-prop:value]" which puts

--custom-prop: value;

in an external style sheet, but that approach of course doesn't work for dynamic values.

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