packages
packages copied to clipboard
[flutter_markdown] Pass parent TextStyle down to MarkdownElementBuilder.visitElementAfter
The parent TextStyle should be passed down to the MarkdownElementBuilder.visitElementAfter method to allow custom markdown tags to override only part of the text style, e.g. the color, but keep all the rest of the styles the same.
This is especially useful when trying to color markdown headers in a certain color, as the parent font size, font family, etc. all are passed down and can be kept, while only the color is overridden.
This will unfortunately lead to a breaking change, due to the nature of how the class is typically used. As all usages of the class are sub-classes any change to the method schema will result in a breaking change!
Enables the following https://github.com/flutter/flutter/issues/105571
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format.) - [x] I signed the CLA.
- [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences] - [x] I listed at least one issue that this PR fixes in the description above.
- [x] I updated
pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.mdto add a description of the change, following repository CHANGELOG style. - [x] I updated/added relevant documentation (doc comments with
///). - [ ] I added new tests to check the change I am making, or this PR is test-exempt.
- [ ] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
@domesticmouse, how do we deal with the breaking change of the PR? Can I just bump the version to the next breaking version, or do you want to consolidate multiple breaking changes into one major release?
@domesticmouse, how do we deal with the breaking change of the PR?
Can I just bump the version to the next breaking version, or do you want to consolidate multiple breaking changes into one major release?
@stuartmorgan thoughts?
I've also exposed now the BuildContext in the visitElementAfter method if you need to access the context for some reason, you no longer have to fallback to workarounds like this:
Which is a very hacky workaround and has it's own drawbacks 🙈
@override
Widget visitElementAfter(md.Element element, TextStyle? preferredStyle) {
return RichText(
text: TextSpan(
children: [
WidgetSpan(
child: Builder(
builder: (context) {
return Text(
innerText,
style: preferredStyle
);
},
),
),
],
),
);
}
Feels free to leave any suggestions/feedback/change requests!
@domesticmouse, how do we deal with the breaking change of the PR? Can I just bump the version to the next breaking version, or do you want to consolidate multiple breaking changes into one major release?
@stuartmorgan thoughts?
Does it have to be a breaking change? What about a new method with a default implementation that calls the existing method (discarding the extra parameters)?
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).
If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?
Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.
@stuartmorgan thanks for the hint, I've just implemented it, not sure yet about the name of the new function 🤔
/cc @domesticmouse
@domesticmouse if you find the time to give this a review here that would be awesome :)
It's no longer a breaking change so should be easier to merge
@IchordeDionysos Are you still planning on updating this based on the last round of feedback?
I'm making this a draft just to get it off of the review queue; please mark it as ready for review once you've had a chance to update. Thanks!
@tarrinneal to adopt, per triage meeting.
Closing and moving changes over to https://github.com/flutter/packages/pull/4393 as I cannot make changes to this pr.