Add new Text components #148
Conversation
arthurxavierx
force-pushed
the
arthur/v2/text2
branch
from
1d9d907 to
94834ac
Compare
arthurxavierx
changed the title
arthurxavierx
force-pushed
the
arthur/v2/text2
branch
from
94834ac to
361f038
Compare
|
|
||
| -- | The `c` type parameter lets us constrain the type of component to which | ||
| -- | a text modifier may be applied: `Text`, `Header` or any. | ||
| type TextModifier c = forall r. PropsModifier (component :: c, type :: Maybe TextType | r) |
There was a problem hiding this comment.
This is neat, I like this.
There was a problem hiding this comment.
Not sure if this is a good idea now, as we can't use, say, muted or color with onDesktop.
There was a problem hiding this comment.
I still think this is a good idea though, even if it means changing onDesktop or using a different pattern there
There was a problem hiding this comment.
Not sure if we can change onDesktop to fit this pattern given how the types work there. This means that props is (component :: c, type :: Maybe TextType), but onDesktop needs to be polymorphic on props, otherwise it can't provide the default values needed.
| , row | ||
| $$$ [ T.paragraph | ||
| $ T.emphasized | ||
| $ _flex |
There was a problem hiding this comment.
_flex maybe belongs on Box the way muted is tied to Text, but we can look into that in a separate PR
There was a problem hiding this comment.
Not sure it belongs on Box. I believe flex may be applied to all block-level elements, and paragraph is a block element. Maybe we should start encoding element types somehow in the library?
arthurxavierx
force-pushed
the
arthur/v2/text2
branch
from
9bb1d97 to
b48850c
Compare
Addressed the comments in #146 and changed the approach used for implementing the text modifiers. I've decided to create a new PR so that the old PR doesn't get too cluttered now that I've changed the implementation basis.