-
-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add RichText component #875
Conversation
The way I'm doing this seems a bit foolish as every tree concatenation eventually becomes a list concatenation in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome @SirFurness! The code looks mostly great, but I do have a few general suggestions:
-
It would be nice to have a separate
RichText
model that is not bound to the component, so that rich text can be manipulated without bringing in the whole UI layer, and even so that alternative renderers may be implemented. We might want to have one that can embed rich text using immediate mode in a<Canvas>
for example. -
It would also be nice to have a DSL for creating rich text. Something like
RichText.(text(~family="Arial", "Hello ") ++ text(~size=24, "world") |> color(Colors.red))
. That is, to be able to optionally set properties at creation, and also be able to set them across a span of text nodes. -
You might want/have to adapt this to feat(components): move font styling out of Style and use families #879
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Aside from a few stylistic suggestion below, I have a couple more general suggestions:
-
As a user,
RichTextModel
is what you're mostly going to use to work with the model. The component is just used to render the end result. Therefore it seems more convenient to renameRichTextModel
toRIchText
and the component toRichTextView
so that the one that's most frequently used is also the shortest and most convenient to use. -
The model isn't really UI-specific (or specific to this UI). Could it be moved to
Core
instead? -
It would be great to have an interface file for
RichTextModel
.
I tried moving
|
Ah, I see, that's unfortunate. Thanks for trying though! |
This reverts commit 17e5445.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, looks good to me now at least!
Thanks for doing an awesome job with this @SirFurness! 🎉 |
ping @thangngoc89 😄 |
Adds a basic
RichText
component. Closes #654It also adds a simple example.
Having different font sizes causes problems with alignment.
alignItems(`Baseline)
would solve this but I don't think that currently exists here.