-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Rename TextBlock to TextLayout #15797
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
Conversation
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.
I prefer this naming. I think this is much clearer: it's more adjective-y, rather than being a distinct noun.
From my perspective this should still be a "driving concept" with a noun. |
But if we commit to this just being "text layout configuration" from a user perspective, this rename is fine |
Right now the requires are
What if we rearrange like this
It feels quite wrong to me to make a |
Hmm that would have conceptual / ergonomic implications when trying to use the "TextBlock" concept (in your proposal commands.spawn((Node, ComputedTextBlock, TextLayout::new_with_justify(JustifyText::Center)) Vs commands.spawn((Node, TextBlock::new_with_justify(JustifyText::Center)) That being said, with the current impl this standalone usage is not currently possible, and I'm not really sure we even need it. In the short term I'm cool with TextLayout, provided it is used "just" for config. I agree that the introduction of a "non layout" concept does muddle things when that new concept isn't currently actually used anywhere outside of layout. If we later decide that TextBlock is a useful user facing concept, we can re-open the discussion. |
Objective
Solution
TextBlock
toTextLayout
.