Phantom border #412
Phantom border #412
Conversation
owanturist
force-pushed
the
phantom-border
branch
from
5c0e94b
to
793ecc8
Compare
There was a problem hiding this comment.
@rtfeldman there are some points which I need for you comment/opinion, could you pay little bit more attention for those please?
src/Css.elm
Outdated
| -} | ||
| wavy : Value { provides | wavy : Supported } | ||
| wavy = | ||
| Value "wavy" |
There was a problem hiding this comment.
I've removed wavy value, because it's related with text-decoration-style which hasn't implemented yet and I thought it can confuse a contributor who will implement it. Should we keep changes or come wavy back?
There was a problem hiding this comment.
I think it's fine to leave it. It already happens that implementors run into things that have already been implemented (e.g. left is reused in several places), so I think this should be fine!
|
|
||
| ## Border Color | ||
|
|
||
| @docs borderColor, borderColor2, borderColor3, borderColor4, borderTopColor, borderRightColor, borderBottomColor, borderLeftColor |
There was a problem hiding this comment.
I'm little bit worry about the docs structure but this kind of approach the most logical and clear by my opinion.
There was a problem hiding this comment.
This grouping seems fine to me! 👍
src/Css.elm
Outdated
| type Supported | ||
| = Supported | ||
| type alias Supported = | ||
| Never |
There was a problem hiding this comment.
Little proposition for making phantom types more phantom :) Like never before.
There was a problem hiding this comment.
Actually there are more variants:
- If we don't want that user will describe phantom types of
Valuewith bothNeverandSupportedtype Supported = Supported Never
- We may not use any special type, just empty tuple. Error messages keep same.
dotted : Value { provides | dotted : () } dotted = Value "dotted"
There was a problem hiding this comment.
A nice feature of having Supported be a union type that doesn't expose its constructor is that other modules can't instantiate it. This limits how other modules can use it, which in this case is desirable!
On the other hand, Never and () can be instantiated outside this module (by using Basics.never and (), respectively), which means we're potentially giving up guarantees around how these values can be constructed, without necessarily knowing what those guarantees are are or why we're giving them up. 😅
To make a long story short, I definitely want to leave this as-is! 😄
There was a problem hiding this comment.
Anyway we will never expose constructor of Value. It means that nobody can reproduce something like that Value "transparent" event we expose Supported(..)
owanturist
force-pushed
the
phantom-border
branch
from
e49d185
to
1f9b26a
Compare
|
@rtfeldman issues have been completed |
|
|
||
| borderStyle ridge | ||
|
|
||
| Similar to `groove`, but reverses the color values in a way that makes the element appear raised. |
There was a problem hiding this comment.
Love the concise descriptions on these! 😻
Hi @rtfeldman! I'm glad to show
borderproperties as a part of #392 here. Please take a look.Checklist
Valueis an open record with a single field. The field's name is the value's name, and its type isSupported. For examplefoo : Value { provides | foo : Supported }Styleaccepts a closed record ofSupportedfields, which always includesinherit,initial, andunsetbecause all CSS properties support those three values! For example,borderFoo : Value { foo : Supported, bar : Supported, inherit : Supported, initial : Supported, unset : Supported } -> Stylephantom-typesbranch, notmaster!