Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
davesnx
force-pushed
the
main
branch
3 times, most recently
from
8b0272d to
d27fb86
Compare
There was a problem hiding this comment.
Amazing PR @zakybilfagih.
Left a few comments worth taking a look, but the rest looks very nice
| //let unsupportedValue = (parser, property) => | ||
| // transform_with_variable( | ||
| // parser, | ||
| // (~loc as _, _) => raise(Unsupported_feature), | ||
| // (~loc, arg) => [[%expr [%e property(~loc)]([%e arg])]], | ||
| // ); |
| (CSS.textDecoration2 | ||
| ~line:(CSS.Types.TextDecorationLine.Value.make ~lineThrough:true ()) | ||
| ()) | ||
| "-webkit-text-decoration: line-through auto solid currentColor; \ | ||
| text-decoration: line-through auto solid currentColor;") |
There was a problem hiding this comment.
right, add another test for this one
|
|
||
| let text_decoration = | ||
| monomorphic( | ||
| polymorphic( |
There was a problem hiding this comment.
yup, this should be mono
There was a problem hiding this comment.
oops, turns out it's a lot simpler using poly since we're generating the shorthand
| ); | ||
|
|
||
| let render_single_animation = | ||
| let render_single_animation = (~loc) => |
| | Error(`Invalid_value(_)) | ||
| | exception (Invalid_value(_)) => Error(`Invalid_value(value)) |
There was a problem hiding this comment.
In terms of behaviour, does this change anything?
There was a problem hiding this comment.
Added the Error(`Invalid_value(_)) pattern to handle the error from render_to_expr, I think we forgot to handle this when refactoring this line
The exception Invalid_value pattern is redundant though, since we transformed it already on render_to_expr.
There was a problem hiding this comment.
As for the Invalid_value payload, I think I removed it at that time because it's a bit ugly lol, just pushed a commit to make it nicer.
| | `webkitFlex | ||
| | `webkitInlineBox | ||
| | `webkitInlineFlex ] | ||
| but an expression was expected of type Css_types.Display.t |
There was a problem hiding this comment.
The issue with aliasing everything is that the error is less clear (since not all polyvariants) are rendered. WDYT?
Do you think is worth it?
There was a problem hiding this comment.
I think this is because the explicit type annotation on the toString method, I think maybe remove it for some properties with flat or simple polyvariant? The only problem, I think, is that it can get pretty long, esp if there is Length.t and Calc.t
2 participants
mask-position: bottom 10px right<position>