Skip to content
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

feat: Add TextElementComponent #2694

Merged
merged 4 commits into from Sep 2, 2023
Merged

feat: Add TextElementComponent #2694

merged 4 commits into from Sep 2, 2023

Conversation

luanpotter
Copy link
Member

@luanpotter luanpotter commented Aug 31, 2023

Description

Add TextElementComponent

Example:

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

@luanpotter luanpotter force-pushed the luan.element-component branch 4 times, most recently from 97f0583 to 2839775 Compare August 31, 2023 04:58
Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, except possibly the ElementComponent name

doc/flame/rendering/text_rendering.md Outdated Show resolved Hide resolved
luanpotter added a commit that referenced this pull request Sep 2, 2023
…docs (#2700)

This occurred to me after a discussion on the [new FCS component
PR](#2694 (comment)).
As per usual, @spydon has opened my eyes to the ultimate truth:

We should rename loads of files, and it shall affect almost no one.

The idea is to (1) add a "Text" prefix to all text-rendering-related
classes and (2) rename the existing `Text*` to `InlineText*` (which is
what they are).

This PR is a bit big, but the changes should hopefully be simple to
review, and can be broken down into:

* Add a proper base class for the node inheritance chain, call it
TextNode (while working on Flame Markdown I realized the value this will
have to me)
* Rename the old TextNode to InlineTextNode
* Rename DocumentNode to DocumentRoot because it is not a node
* Rename Element to TextElement
* Rename the old TextElement to InlineTextElement
* Rename Style to FlameTextStyle (note: we could consider dropping the
Flame here)
* Rename the old FlameTextStyle to InlineTextStyle
* Update the docs accordingly
* Add some more diagrams and explanations to the docs, following the new
nomenclature
* I also updated our "internal" imports to use the text module to make
life so much easier (this could arguably be done in a separate PR, but I
honestly think it's easier to review together, please lmk if you prefer
me to split).

These are all breaking changes but likely won't actually affect most
users (see below).

While this is breaking, it should hopefully not affect most users,
because these are all infrastructure classes that most people aren't
using directly. If you are using the FCS components, or the renderers
`TextPaint` or `SpriteFontRenderer` directly, this should have zero
effect to you.

If you are using the Nodes, Stlyes or Elements directly, or have a
custom TextRenderer, see below.

Migrating should be a simple matter of renaming your type references:

* from TextNode to InlineTextNode
* from TextElement to InlineTextElement
* from Element to TextElement
* from FlameTextStyle to InlineTextStyle
* from Style to FlameTextStyle

Make sure to do it in the appropriate order not to cause any
double-replace issues.

If you are importing via the module `package:flame/text.dart`, which we
highly encourage, you should not have to change any import statements
whatsoever.
@luanpotter luanpotter changed the title feat: Add ElementComponent and improve docs for text rendering feat: Add ElementComponent Sep 2, 2023
@luanpotter luanpotter changed the title feat: Add ElementComponent feat: Add TextElementComponent Sep 2, 2023
Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

@spydon spydon enabled auto-merge (squash) September 2, 2023 19:26
@spydon spydon merged commit 10fb65f into main Sep 2, 2023
7 checks passed
@spydon spydon deleted the luan.element-component branch September 2, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants