Repository navigation
Project structure and naming conventions #93
Description
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:
- 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.Displaythere areFloat,Position,Display,Overflow,Visibility,Cursoretc (in a single module). This is what we have right now. - 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:
- In Clay they aren't prefixing property values with a property name.
For example, itsabsoluteinstead ofpositionAbsolute.
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 inCSS.purswe'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.leftAs 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.
Activity
garyb commentedon Jul 7, 2018
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
CSSmodule would export properties, units, and basically everything else would be expected to import as qualified - so yeah, dropping prefixing of the names for things likeFloatandCleartoo.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.
vyorkin commentedon Jul 7, 2018
Great! So if you're ok with accepting breaking changes I can start the work and open a PR with your suggestions.
garyb commentedon Jul 7, 2018
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
Writerbased API performs very well... that's a separate thing I could look at though.vyorkin commentedon Jul 8, 2018
By using arrays you mean API similar to this (?)
(picture was taken from the PureScript by Example book, the chapter about building DSL's)
e.g.
Or something completely different?
garyb commentedon Jul 8, 2018
Even simpler actually:
Currently we have
type CSS = StyleM UnitwhereStyleMis aWriter (Array Rule) a. My suggestion is just to havetype CSS = Array Ruledirectly instead.vyorkin commentedon Jul 9, 2018
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?
safareli commentedon Jul 10, 2018
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 commentedon Jul 10, 2018
Is it hard to write Halogen HTML? No. It'd be exactly the same here. 😄
safareli commentedon Jul 10, 2018
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 commentedon Jul 10, 2018
I don't think a
Writerbased 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. 🙂natefaubion commentedon Jul 11, 2018
FWIW, https://github.com/paulyoung/purescript-style is a WIP implementation based on Variant that uses arrays.