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

Add ReadMoreText.rich constructor #59

Merged
merged 2 commits into from
Mar 31, 2024

Conversation

maRci002
Copy link
Collaborator

closes #38

This PR enables the direct use of TextSpan instead of a plain String. In this case, annotations won't be triggered since the developer has total control over styling and adding interactions to parts of the text.

@maRci002
Copy link
Collaborator Author

@jonataslaw, TrimMode.Length works fine; however, TrimMode.line sometimes displays the 'Show more' button on the next line. I suspect the issue lies with DefaultTextStyle — it seems something is inherited during the text rendering but not when laying out to measure it.

Any ideas?

Text layout measurement should work fine since annotations already create a custom TextSpan, so this one should work fine as well.

        final dataTextSpan = widget.richData ??
            _buildAnnotatedTextSpan(
              data: widget.data!,
              textStyle: effectiveTextStyle,
              regExp: regExp,
              annotations: widget.annotations,
            );

@maRci002 maRci002 changed the title [WIP] Add ReadMoreText.rich constructor Add ReadMoreText.rich constructor Mar 30, 2024
@maRci002 maRci002 marked this pull request as ready for review March 30, 2024 11:12
@maRci002
Copy link
Collaborator Author

The issue was with Text.rich, a higher-level widget that may introduce additional fields to RichText. However, in this case, we needed the rendering/layout to be perfectly identical.

@maRci002
Copy link
Collaborator Author

I need to check whether DefaultTextStyle is respected

@maRci002
Copy link
Collaborator Author

PR is ready for review

effectiveTextStyle = defaultTextStyle.style.merge(widget.style);
} else {
Copy link
Owner

Choose a reason for hiding this comment

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

It is a regression of #35 ?
I couldn't test to be sure if this brings back the problem, but we need to double check this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

final defaultTextStyle = DefaultTextStyle.of(context);
var effectiveTextStyle = widget.style;
if (widget.style == null || widget.style!.inherit) {
  effectiveTextStyle = defaultTextStyle.style.merge(widget.style);
}

The effectiveTextStyle will never be null:

  • If widget.style is null, then effectiveTextStyle will be equal to defaultTextStyle.
  • If widget.style has inherit set to true, effectiveTextStyle will be a merge of defaultTextStyle and widget.style.
  • If widget.style has inherit set to false, effectiveTextStyle will be set according to widget.style and will not be overwritten by defaultTextStyle.

But I rewrote logic to make it more explicit:

final defaultTextStyle = DefaultTextStyle.of(context);
TextStyle effectiveTextStyle;
if (widget.style == null || widget.style!.inherit) {
  effectiveTextStyle = defaultTextStyle.style.merge(widget.style);
} else {
  effectiveTextStyle = widget.style!;
}

@jonataslaw
Copy link
Owner

Wow, it is amazing!

@jonataslaw
Copy link
Owner

LGTM

@jonataslaw jonataslaw merged commit 68b749f into jonataslaw:master Mar 31, 2024
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.

Add support for RichText using TextSpan
2 participants