[flutter_markdown] Pass parent TextStyle down to MarkdownElementBuilder.visitElementAfter by IchordeDionysos · Pull Request #3281 · flutter/packages

IchordeDionysos · 2023-02-23T17:47:19Z

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 flutter/flutter#105571

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

IchordeDionysos · 2023-02-23T17:48:42Z

@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 · 2023-02-23T20:56:08Z

@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?

IchordeDionysos · 2023-02-24T13:32:07Z

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 🙈

@overrideWidgetvisitElementAfter(md.Element element, TextStyle? preferredStyle) {
returnRichText(
text:TextSpan(
children: [
WidgetSpan(
child:Builder(
builder: (context) {
returnText(
innerText,
style: preferredStyle
);
},
),
),
],
),
);
}

Feels free to leave any suggestions/feedback/change requests!

stuartmorgan-g · 2023-02-24T19:38:41Z

@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)?

flutter-dashboard · 2023-03-07T13:28:53Z

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.

IchordeDionysos · 2023-03-07T13:56:05Z

@stuartmorgan thanks for the hint, I've just implemented it, not sure yet about the name of the new function 🤔

/cc @domesticmouse

IchordeDionysos · 2023-03-21T17:11:55Z

@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

stuartmorgan-g

CI is failing the autoformat check, so the code will need to be autoformatted.

stuartmorgan-g · 2023-04-13T16:05:39Z

-## NEXT
+## 0.6.15

+* Introduce a new `MarkdownElementBuilder.visitElementAfterWithContext()` method passing the widget `BuildContext` and


"Introduces"
See the CHANGELOG style guide linked from the PR descirption.

stuartmorgan-g · 2023-04-13T16:06:15Z


+* Introduce a new `MarkdownElementBuilder.visitElementAfterWithContext()` method passing the widget `BuildContext` and
+ the parent texts' `TextStyle`. 
+ This should allow custom syntax implementations to get data from the current context, as well as properly style any


The changelog should just describe the change (the first sentence); it doesn't need to provide the motivation for it.

stuartmorgan-g · 2023-04-13T16:06:33Z

+## 0.6.15

+* Introduce a new `MarkdownElementBuilder.visitElementAfterWithContext()` method passing the widget `BuildContext` and
+ the parent texts' `TextStyle`. 


stuartmorgan-g · 2023-04-13T16:09:28Z

+ /// Called when an Element has been reached, after its children have been
+ /// visited.
+ ///
+ /// If [MarkdownWidget.styleSheet] has a style of this tag, will passing


This doesn't parse as a sentence. I'm assuming this should say something like "has a style for this tag, it will be passed as [preferredStyle]."?

stuartmorgan-g · 2023-04-13T16:09:52Z

+ /// If [MarkdownWidget.styleSheet] has a style of this tag, will passing
+ /// to [preferredStyle].
+ ///
+ /// If parent element has [TextStyle]'s set, it will be passed as


"has a [TextStyle] set"

stuartmorgan-g · 2023-05-02T19:45:31Z

@IchordeDionysos Are you still planning on updating this based on the last round of feedback?

stuartmorgan-g · 2023-05-23T19:42:31Z

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!

stuartmorgan-g · 2023-07-06T18:42:59Z

@tarrinneal to adopt, per triage meeting.

tarrinneal · 2023-07-07T00:41:12Z

Closing and moving changes over to #4393 as I cannot make changes to this pr.

…er.visitElementAfter (#4393) 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 flutter/flutter#105571 replaces #3281

…er.visitElementAfter (flutter#4393) 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 flutter/flutter#105571 replaces flutter#3281

Pass parent TextStyle down to MarkdownElementBuilder.visitElementAfter
f2e66c3

IchordeDionysos requested a review from domesticmouse as a code owner February 23, 2023 17:47

github-actionsBot added the p: flutter_markdown label Feb 23, 2023

Add BuildContext to the visitElementAfter method
8f6047d

Update builder.dart
f3770fc

IchordeDionysos added 2 commits March 7, 2023 13:17

Make it a non-breaking change
de053eb

Use underscore instead
22c6b16

flutter-dashboardBot added the needs tests label Mar 7, 2023

IchordeDionysos added 2 commits March 7, 2023 13:29

Bump version & changelog
1315839

Add test for visitElementAfterWithContext
75adc21

Merge branch 'main' into task/pass-parent-to-markdown-element-builder
f40060b

domesticmouse approved these changes Mar 22, 2023
View reviewed changes

stuartmorgan-g self-requested a review April 4, 2023 20:07

stuartmorgan-g reviewed Apr 13, 2023
View reviewed changes

stuartmorgan-g marked this pull request as draft May 23, 2023 19:42

tarrinneal mentioned this pull request Jul 7, 2023
[flutter_markdown] Pass parent TextStyle down to MarkdownElementBuilder.visitElementAfter #4393
Merged
11 tasks

tarrinneal closed this Jul 7, 2023

Uh oh!

Conversation

IchordeDionysos commented Feb 23, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

IchordeDionysos commented Feb 23, 2023

Uh oh!

domesticmouse commented Feb 23, 2023

Uh oh!

IchordeDionysos commented Feb 24, 2023

Uh oh!

stuartmorgan-g commented Feb 24, 2023

Uh oh!

flutter-dashboardBot commented Mar 7, 2023

Uh oh!

IchordeDionysos commented Mar 7, 2023

Uh oh!

IchordeDionysos commented Mar 21, 2023

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-gApr 13, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-gApr 13, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-gApr 13, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-gApr 13, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-gApr 13, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented May 2, 2023

Uh oh!

stuartmorgan-g commented May 23, 2023

Uh oh!

stuartmorgan-g commented Jul 6, 2023

Uh oh!

tarrinneal commented Jul 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

IchordeDionysos commented Feb 23, 2023•
edited
Loading