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.

Sign up for GitHub

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

Jump to bottom

Allow custom blocks to be something other than Column or SizedBox #7859

Merged
merged 6 commits into from
Oct 21, 2024
Merged

Allow custom blocks to be something other than Column or SizedBox #7859

merged 6 commits into from
Oct 21, 2024

Conversation

gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Oct 11, 2024 •
edited
Loading

Description

This adds support for allowing block tags recognized by the Markdown processor to insert something other than just a Column or a SizedBox (the defaults for blocks with children, and without). Without this ability, custom builders can't insert their own widgets to, say, make it be a colored container instead.

This addresses a customer request.

Fixes flutter/flutter#135848

domesticmouse
domesticmouse approved these changes Oct 12, 2024
View reviewed changes
Copy link
Contributor

@domesticmouse domesticmouse left a comment

Choose a reason for hiding this comment

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

LGTM

stuartmorgan
stuartmorgan requested changes Oct 14, 2024
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

This addresses a customer request.

This looks like it's flutter/flutter#135848 ?

packages/flutter_markdown/lib/src/builder.dart Outdated Show resolved Hide resolved
packages/flutter_markdown/lib/src/builder.dart Outdated Show resolved Hide resolved
packages/flutter_markdown/lib/src/builder.dart Outdated Show resolved Hide resolved
@gspencergoog gspencergoog force-pushed the custom_tag_block branch 2 times, most recently from 71b8cc2 to eb68bc0 Compare October 14, 2024 19:15
@gspencergoog
Copy link
Contributor Author

Okay, I've removed the new API surface, and just kept the fixed implementation of builder.visitElementAfter so that it can create custom blocks that are something other than a Column or a SizedBox.

An explanation of the remaining change in visitElementAfter:
Inside the if(_isBlockTag(tag)) in builder.visitElementAfter, we are building a widget for the block element, which may be a container for its block children or as a standalone widget.

In the prior implementation, only a handful of elements get specialized block widgets (e.g., list, table, blockquote, pre, hr, see the conditional statements after the modified code), other block elements and custom blocks will be built as a Column or an empty SizedBox, which may not be ideal in many use cases. For example, there is no way to define a custom block that hosts all its children in a pink container.

With the added code, we will visit the custom block element's visitElementAfterWithContext to build the block widget.

@gspencergoog gspencergoog changed the title Add support for extending the list of allowed customizable block tags Allow custom blocks to be something other than Column or SizedBox Oct 14, 2024
@gspencergoog gspencergoog changed the title Allow custom blocks to be something other than Column or SizedBox Allow custom blocks to be something other than Column or SizedBox Oct 14, 2024
@gspencergoog
Copy link
Contributor Author

I think this does fix the last item in flutter/flutter#135848, referenced in flutter/flutter#135848 (comment), so I marked it as fixing that bug.

stuartmorgan
stuartmorgan approved these changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of adjustments to the code (one optional).

packages/flutter_markdown/lib/src/builder.dart Outdated Show resolved Hide resolved
packages/flutter_markdown/lib/src/builder.dart Outdated Show resolved Hide resolved
packages/flutter_markdown/lib/src/builder.dart Outdated Show resolved Hide resolved
@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 21, 2024
@auto-submit auto-submit bot merged commit aad3fd0 into flutter:main Oct 21, 2024
76 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 22, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 22, 2024
@engine-flutter-autoroll
aad3fd00f Allow custom blocks to be something other than Column or …
c4f6ef1 
…`SizedBox` (flutter/packages#7859)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 22, 2024
@engine-flutter-autoroll
Roll Packages from b6f7e47c7586 to 5e03bb1da411 (3 revisions) (#157349)
9d27d52 
flutter/packages@b6f7e47...5e03bb1

2024-10-21 43054281+camsim99@users.noreply.github.com [video_player_android] Correct rotation of videos recorded by the camera (flutter/packages#7846)
2024-10-21 gspencergoog@users.noreply.github.com Allow custom blocks to be something other than `Column` or `SizedBox` (flutter/packages#7859)
2024-10-21 49699333+dependabot[bot]@users.noreply.github.com [camera]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/camera/camera_android/android (flutter/packages#7905)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
M97Chahboun pushed a commit to M97Chahboun/flutter that referenced this pull request Oct 30, 2024
@engine-flutter-autoroll @M97Chahboun
Roll Packages from b6f7e47c7586 to 5e03bb1da411 (3 revisions) (flutte…
25e7b9f 
…r#157349)

flutter/packages@b6f7e47...5e03bb1

2024-10-21 43054281+camsim99@users.noreply.github.com [video_player_android] Correct rotation of videos recorded by the camera (flutter/packages#7846)
2024-10-21 gspencergoog@users.noreply.github.com Allow custom blocks to be something other than `Column` or `SizedBox` (flutter/packages#7859)
2024-10-21 49699333+dependabot[bot]@users.noreply.github.com [camera]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/camera/camera_android/android (flutter/packages#7905)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan stuartmorgan stuartmorgan approved these changes

@domesticmouse domesticmouse domesticmouse approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: flutter_markdown
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[flutter_markdown] please allow custom block elements
3 participants
@gspencergoog @domesticmouse @stuartmorgan