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

[flutter_markdown] please allow custom block elements #135848

Open
2 tasks done
sma opened this issue Oct 2, 2023 · 8 comments
Open
2 tasks done

[flutter_markdown] please allow custom block elements #135848

sma opened this issue Oct 2, 2023 · 8 comments
Labels
c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter p: flutter_markdown flutter/packages flutter_markdown P3 Issues that are less important to the Flutter project package flutter/packages repository. See also p: labels. team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team

Comments

@sma
Copy link

sma commented Oct 2, 2023

Is there an existing issue for this?

Use case

I tried to add my own custom block element using a blockSyntaxes: const [NoteSyntax()], declaration in the Markdown widget. My NoteSyntax creates an Element('note', children) which then eventually gets passed to the visitor that creates widgets.

I tried to add my own builder using builders: {'note': NoteBuilder()}, in the Markdown widget, but this is never called because MarkdownBuilder.visitElementBefore uses a private function _isBlockTag to determine whether the provided Element is a block element by comparing its tag to a hardcoded list of tags.

Therefore, it incorrectly thinks my Element is an inline element and crashes 1 because _addParentInlineIfNeeded wants add an inline element that requires a tag but the parent of my block node is the document which has no tag.

Proposal

I'd suggest to dispatch the isBlock call to the element itself – or let me extend that list.

Footnotes

  1. See line 632 of builder.dart, style: styleSheet.styles[tag!],.

@dam-ease dam-ease added the in triage Presently being triaged by the triage team label Oct 2, 2023
@dam-ease
Copy link

dam-ease commented Oct 2, 2023

Hi @sma. Thanks for filing this!

I tried to add my own builder using builders: {'note': NoteBuilder()}, in the Markdown widget, but this is never called because MarkdownBuilder.visitElementBefore uses a private function _isBlockTag to determine whether the provided Element is a block element by comparing its tag to a hardcoded list of tags.

Can you share a completed and minimal reproducible code sample that we can use to reproduce this.
Also, this might be similar to #127425, can you check it out??

@dam-ease dam-ease added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Oct 2, 2023
@sma
Copy link
Author

sma commented Oct 2, 2023

#127425 has the same reason: An Element tagged section isn't detected as a block element. It's probably this one.

If I add 'section' to the _kBlockTags, it works:

image

Regarding my problem, here's a minimal example that work if I add 'note' to _kBlockTags and which fails otherwise. Hence my suggestion to allow for custom block elements instead of hardcoding them.

import 'package:flutter/material.dart';
import 'package:flutter_markdown/flutter_markdown.dart';

// ignore: depend_on_referenced_packages
import 'package:markdown/markdown.dart' as md;

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: Markdown(
          data: '# Hallo Welt\n[!NOTE] Dies ist eine wichtige Nachricht.',
          // data: 'Foo[^1] bar.\n\n[^1]: Baz.',
          blockSyntaxes: const [NoteSyntax()],
          builders: {'note': NoteBuilder()},
        ),
      ),
    );
  }
}

class NoteBuilder extends MarkdownElementBuilder {
  // super class forgot to declare a const constructor
  // const NoteBuilder();

  @override
  Widget? visitText(md.Text text, TextStyle? preferredStyle) {
    return Container(color: Colors.red, child: Text(text.text, style: preferredStyle));
  }
}

class NoteSyntax extends md.BlockSyntax {
  const NoteSyntax();

  @override
  md.Node? parse(md.BlockParser parser) {
    final line = parser.current;
    parser.advance();
    return md.Element('note', [md.Text(line.content.substring(8))]);
  }

  @override
  RegExp get pattern => RegExp(r'^\[!NOTE] ');
}

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Oct 2, 2023
@dam-ease dam-ease added c: new feature Nothing broken; request for a new capability package flutter/packages repository. See also p: labels. c: proposal A detailed proposal for a change to Flutter team-ecosystem Owned by Ecosystem team p: flutter_markdown flutter/packages flutter_markdown and removed in triage Presently being triaged by the triage team labels Oct 3, 2023
@dawidope
Copy link
Contributor

dawidope commented Oct 4, 2023

Hi, you are right about section tag. It was the main issue in #127425.

Element object doesn't know if it's a block. I don't see any way to make a connection between BlockSyntax and MarkdownElementBuilder right now. I mean that the MarkdownBuilder has to receive the information about the type from the developer. I can propose two solutions:

  1. Add to MarkdownElementBuilder's constructor isBlock parameter.
  2. Split builders into inLineBuilders and blockBuilders.

I can create PR when we find out the best solution.

@stuartmorgan stuartmorgan added P2 Important issues not at the top of the work list triaged-ecosystem Triaged by Ecosystem team labels Oct 5, 2023
@stuartmorgan
Copy link
Contributor

/cc @tarrinneal

@Balin-P
Copy link

Balin-P commented Oct 27, 2023

Any update on this? Trying to build a custom list block that can use different bullet points per level, but this doesn't seem possible with the way the code is currently structured. Getting the same error where it thinks my custom block element is an inline one.

@Balin-P
Copy link

Balin-P commented Nov 1, 2023

Managed to fix this issue for myself by adding an extra parameter to MarkdownBuilder which is a List of Strings that gets added to kBlockTags when _isBlockTag (which I also moved into the class) gets called.

Also had to add a call to check the builders for my custom tag in the visitElementAfter method of MarkdownBuilder as there didn't seem to be any way of it doing that on its own.

@MaxSchilling

This comment was marked as off-topic.

@mrares

This comment was marked as off-topic.

@stuartmorgan stuartmorgan added P3 Issues that are less important to the Flutter project and removed P2 Important issues not at the top of the work list labels Feb 15, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this issue Mar 19, 2024
Fixes problem with adding custom block syntax.

Issue: flutter/flutter#135848
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter p: flutter_markdown flutter/packages flutter_markdown P3 Issues that are less important to the Flutter project package flutter/packages repository. See also p: labels. team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team
Projects
None yet
Development

No branches or pull requests

7 participants