Introduce a Css.Property module?

[rtfeldman]

Problem

Some CSS property names are also value names. For example, there's the left property (e.g. left: 5px) and the left value (e.g. float: left).

In elm-css, properties are functions and values use open records to deal with how many CSS values are overloaded for different properties.

It's not possible to have a function that's also an open record, which means we can't have a property (function) named left as well as a value (open record) named left in the same module.

Potential Solutions

Today we prioritize the function, e.g. left is always a function, and properties that support values with namespace conflicts like this have worse types (e.g. float).

I haven't been happy with this solution in practice, and so the phantom-types branch is using underscores to avoid the collision - for example having left for the function and left_ for the value.

@stoeffel proposed another potential solution, which I'd like to discuss here: introduce a Css.Property module which would hold all the property functions. Then we could have Css.left for the open record and Css.Property.left for the function.

Tradeoffs

1. As far as I can tell, this would address the namespace collision without using underscores or worse types.
2. In practice, almost all modules that are currently using import Css would also need to import Css.Property.
3. Any file which used import Css exposing (..) as well as import Css.Property exposing (..) could run into ambiguous names which would have to be resolved by calling in a qualified way, e.g. Css.left or Css.Property.left.

I'm leaning towards adopting this solution, and doing it on the phantom-types branch since that'll be a breaking API change anyway.

[pokorson]

I've started using elm-css ~2 weeks ago and indeed name collision between attribute names and values was a bit annoying. If it's possible to make transition from Vanilla CSS to elm-css smoother that's great 😄

[edkv]

I'm against this - I think this solution would complicate the API a lot.

How many problematic values do we have? flex, left, right, top, bottom, content, text, color - that's all? Not so many! Also I think most of them are rarely used: top/bottom (for verticalAlign), content (for flexBasis), text (for backgroundClip) and color (for backgroundBlendMode). Moreover, if you use flexbox, you probably don't need float and left/right that much. So is it really worth it?

I don't see any serious problems with underscores. Maybe it'd better to just add some extra docs to the top of the Css module to explain the issue and list all such values there? Users would need to learn this only once but then they wouldn't need to import Css.Property everywhere. Also, new type signatures are very helpful as they show things like left_ and right_ so it's easy to figure out what the issue is if you forget about it:

float :
    Value
        { none : Supported
        , left_ : Supported
        , right_ : Supported
        , inherit : Supported
        , initial : Supported
        , unset : Supported
        }
    -> Style

Also I write my CSS the following way:

[ Css.display Css.block
, Css.width (Css.px 100)
, Css.backgroundColor (Css.rgb 0 0 0)
]

This allows to avoid exposing (..), keep imports short and simple, and not to ever think about name collisions with other modules. But with the proposed changes I would need to do something like

import Css.Property as Prop

[ Prop.display Css.block
, Prop.width (Css.px 100)
, Prop.backgroundColor (Css.rgb 0 0 0)
]

which makes it less readable. Why do I need to write Css.block but Prop.backgroundColor? Isn't backgroundColor also a part of CSS? :smile:

[sporto]

Could values be union types? e.g. display Block

[stoeffel]

Could values be union types? e.g. display Block

That doesn't work, because we want the constraint that only certain values can be used with certain properties. Meaning backgroundColor Block shouldn't compile.

[rtfeldman]

This allows to avoid exposing (..), keep imports short and simple, and not to ever think about name collisions with other modules. But with the proposed changes I would need to do something like

import Css.Property as Prop

[ Prop.display Css.block
, Prop.width (Css.px 100)
, Prop.backgroundColor (Css.rgb 0 0 0)
]

which makes it less readable. Why do I need to write Css.block but Prop.backgroundColor? Isn't backgroundColor also a part of CSS? 😄

Doing import Html.Attributes as Attr is basically the same thing.

Do you think that's been a big burden in elm/html?

[edkv]

Doing import Html.Attributes as Attr is basically the same thing.

Do you think that's been a big burden in elm/html?

@rtfeldman Actually, maybe it would be great to have everything (including Events, Keyed and Lazy) under single Html namespace. I just checked and the only problematic value is form which can be both an element and an attribute. Other conflicting names are map, one of which can be renamed to mapAttribute, and Keyed.node, which can be renamed to Html.keyedNode (also, Keyed.ul and Keyed.ol can just be removed).

But it's not the same thing really. Html elements are usable without attributes so you can just import Html when you don't need attributes, and in CSS properties don't exist without values so with the proposed solution users would always need to import two modules if they want to do anything useful. Also, in the case of HTML, elements are "more important" than attributes so we have them in the main Html module and attributes are in the secondary one. And in CSS I would say that properties are "more important" than values, so maybe it's better to have Css.display Css.Values.block? (I don't think it is)

I just love how simple it is that we have most of the things under single Css namespace. A few functions with an underscore seem like a better solution to me than introducing a new module and moving half of all stuff into it.

3. Any file which used import Css exposing (..) as well as import Css.Property exposing (..) could run into ambiguous names which would have to be resolved by calling in a qualified way, e.g. Css.left or Css.Property.left.

And I don't think this looks better than left and left_.

Also, another idea for simple properties:

[ display.flex
, float.left
, verticalAlign.top
]

Although I don't like it much.

[wolfadex]

I haven't been using elm for very long, but I very much appreciate it's verbosity. I think both newcomers to elm and seasoned devs would benefit more from the code being self documenting than having to refer to documentation to know the difference between Css.left vs Css.left_.

[edkv]

If we didn't have issues with name collisions or had a way to use same names both for properties and values, would we still need to introduce a Css.Property module? Probably no. Both solutions - Css.Property and underscores - are tradeoffs and would make the API worse than it would be if we didn't need them. The question is - which of the solutions is the lesser evil? For me it's underscores.

[rtfeldman]

new type signatures are very helpful as they show things like left and right so it's easy to figure out what the issue is if you forget about it:

float :
    Value
       { none : Supported
       , left_ : Supported
       , right_ : Supported
       , inherit : Supported
       , initial : Supported
       , unset : Supported
       }
   -> Style

This is a good point, and one I hadn't thought of.

On the current phantom-types branch, if you say float left you'll get an error saying "float doesn't accept left" and then it will show you all the things float does accept, including left_. So the compiler error tells you the problem as well as the fix; you don't need to go to the docs.

I also thought of another potential problem with the "separate module" approach:

[ Prop.display Css.block
, Prop.width (Css.px 100)
, Css.hover [ Prop.textDecoration Css.underline ]
, Prop.backgroundColor (Css.rgb 0 0 0)
]

Not everything that returns a Style is a CSS property. I would be surprised to find pseudo-classes like hover and pseudo-elements like before in a module called Css.Property (since they're not properties), but that means if you're using them in a qualified style like this, you have a mix of Css. and Prop. in the same list of Style values.

This also happens in Html, where you call functions from Html.Attributes and Html.Events to get values that go in a List (Attribute msg), but if we took that approach, we'd have something like Css.PseudoClass.hover and Css.PseudoElement.before, and I don't think most elm-css users would go looking for those functions in those modules.

Considering all these points, I'm leaning toward giving the underscores approach a try and seeing how people feel about it in practice.