purescript-contrib/purescript-css

Project structure and naming conventions

vyorkin opened this issue ยท 11 comments

I'm adding some CSS props and their values as I discover whats missing so I just want to clarify some conventions to make things that I'm adding look consistent.

Regarding the project structure I see there are (at least) two ways to do it:

  1. Keep the same (or very close) module structure as in Clay. If I get it right they put several CSS properties that are logically related to each other. For example, in Clay.Display there are Float, Position, Display, Overflow, Visibility, Cursor etc (in a single module). This is what we have right now.
  2. Make smaller/granular modules (keep a module per CSS property and its values). I don't know if it makes sense to refactor and split existing modules (will be a breaking change), but maybe it makes sense for new modules (new CSS props).

I prefer the first option, but I'm not sure we have to choose between the two.
For example, in Clay, they keep the cursor property and its values in the Display module:

https://github.com/sebastiaanvisser/clay/blob/54dc9eaf0abd180ef9e35d97313062d99a02ee75/src/Clay/Display.hs#L268
Also some values are prefixed with a property name, like cursorText:
https://github.com/sebastiaanvisser/clay/blob/54dc9eaf0abd180ef9e35d97313062d99a02ee75/src/Clay/Display.hs#L278

I'm about making a PR that adds this prop so I'm thinking about putting it in a separate module because there are too many values and some of them have names like default or text.

Regarding the naming of CSS props and values:

  1. In Clay they aren't prefixing property values with a property name.
    For example, its absolute instead of positionAbsolute.
    Such naming convention sometimes may lead to collisions.
    One way to prevent potential collisions is not to re-export everything from the source module, so in CSS.purs we'll be re-exporting only unique most commonly used CSS properties and values. When we want to use something specific we have to use qualified imports, e.g.:
import CSS.Float as Float

fromString "foo" ? do
  float Float.left

As I can see, this is how its currently done, but with some exceptions, for example, the values of float are prefixed:
https://github.com/slamdata/purescript-css/blob/9e78699ea728f519ec5b3e830aa2036823dd50a7/src/CSS/Display.purs#L125
https://github.com/slamdata/purescript-css/blob/9e78699ea728f519ec5b3e830aa2036823dd50a7/src/CSS/Display.purs#L168
2. Another way (which I dislike for long names) is to always make prefixed values (but keep the old names as is?).

Actually, after writing the naming options above now I think it makes sense to use both (as we have now): not to prefix values if its possible, but sometimes prefix them when its unavoidable and may lead to clashes.

garyb commented

Thanks for all your recent efforts on this, I must admit I procrastinate about looking at these things because it does feel like this library is a bit of a dumping ground, and the reasons you mention here are a big part of it ๐Ÿ˜„

I think breaking stuff to bring some good (or just predictable) naming principles is all good. ๐Ÿ‘

I gave it some thought today, and I think ideally for me, the main CSS module would export properties, units, and basically everything else would be expected to import as qualified - so yeah, dropping prefixing of the names for things like Float and Clear too.

I need to go actually, so that's not a very comprehensive answer, but interested to see your thoughts on that. I have some other suggestions to write up, will do so later, but they're more drastic changes.

Great! So if you're ok with accepting breaking changes I can start the work and open a PR with your suggestions.

garyb commented

Actually, my other suggestion doesn't really do any good ๐Ÿ˜„ - I was going to propose a thing like we do in the Halogen HTML DSL where there are row types that index things, but I don't think that's necessary here since any property can be used on any element, as far I know.

One other thing I'd be happy to change is to do away with is the monadic elements of the DSL though, in favour of using arrays and such. I can't imagine the current Writer based API performs very well... that's a separate thing I could look at though.

in favour of using arrays and such

By using arrays you mean API similar to this (?)

image
(picture was taken from the PureScript by Example book, the chapter about building DSL's)

e.g.

[ color := rgba 0 0 0 0.5
, textAlign := TextAlign.center
, height := rem 4.5
]

Or something completely different?

garyb commented

Even simpler actually:

[ color (rgba 0 0 0 0.5)
, textAlign TextAlign.center
, height (rem 4.5)
]

Currently we have type CSS = StyleM Unit where StyleM is a Writer (Array Rule) a. My suggestion is just to have type CSS = Array Rule directly instead.

got it, I think I can do that, so I'll start working on this after migrating tests as per #30 if you don't mind?

Tho the Monad interface might be much easier to use as you don't need to use [,] and merging other css in is easier then <> (which might also need some $)

garyb commented

Is it hard to write Halogen HTML? No. It'd be exactly the same here. ๐Ÿ˜„

Writer Array kinda thing might be very nice for halogen too actually :D it would make it feel like HAML (indentation based templating language for HTML)

but yah array is fine :p

garyb commented

I don't think a Writer based DSL is a bad thing, just it'd be better as a separate layer, so you have the choice of aethetics/convenience vs better performance. ๐Ÿ™‚

FWIW, https://github.com/paulyoung/purescript-style is a WIP implementation based on Variant that uses arrays.