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

feat: Support custom attributes syntax to allow for multiple styles in the text rendering pipeline #3519

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

luanpotter
Copy link
Member

Description

This is a proposal to extend both our text rendering pipeline and our markdown-parsing capabilities to support more fine-grained styling by leveraging a subset of the extended markdown definition of "custom attributes" (for example, see markdown-it-attrs as an exemplary implementation).

On the markdown side, the syntax will look like this:

[This is a custom class]{.my-custom-class}
This word will be [red]{.red} and this one will be [blue]{.blue}.

Note that the current implementation only supports specifying the class name and will skip other possible permutations.

On the Flame side, we add a new customStyles map to the DocumentStyle class, allowing users to provide as many styles as desired.

Example:

image
image

Currently, users have to hijack the existing syntaxes for the desired styles; with the addition of code and strike-through recently I have doubled the possibilities for custom styles, but the solution still remained a hack. Also I now need more styles, and the only way forward is to support arbitrary styling.

While we could consider supporting other attributes, such as id, css styling, etc, within the attribute syntax, the class name is the most flexible and versatile option, and it already solves all use cases, as class names can be used as ids (if desired) and can be used to apply the styling indirectly. I don't see any immediate advantage to supporting more complex constructs within the attributes block (though of course we can iterate as needed).

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Copy link
Contributor

github-actions bot commented Mar 12, 2025

Benchmark Results

Package flame:

Benchmarks Current Branch
[luan.custom-md]
Base Branch
[main]
Diff
Render Components Benchmark 989.518 μs 964.761 μs 🔴 +2.566 %
Updating Components Benchmark 342521.000 μs 326599.429 μs 🔴 +4.875 %

Benchmarks provided with 💙 by Dart Benchmark Action.

@@ -0,0 +1,110 @@
import 'package:markdown/markdown.dart';

// NOTE: values obtained from file `charcode.dart` from the markdown package
Copy link
Member Author

Choose a reason for hiding this comment

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

sadly we cannot access the file as it is not exported. tbh I do prefer my naming schema though :P

final customClassName = element.attributes['class'];
if (customClassName != null) {
if (element.tag != 'span') {
throw Exception(
Copy link
Member Author

Choose a reason for hiding this comment

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

we could consider support for other inline elements, for example **foo**{.super-bold}, which is part of markdown-it-attrs.
but I think it is unnecessary because you can just make your custom style bold or whatever you want.


// cSpell:ignore charcode.dart (file name from another package)
// NOTE: values obtained from file `charcode.dart` from the markdown package
class _Chars {
Copy link
Member

Choose a reason for hiding this comment

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

Does this even need a class?

Copy link
Member Author

Choose a reason for hiding this comment

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

No but I thought it was more organized to group these together than let them laying around

@luanpotter luanpotter merged commit fbc5805 into main Mar 12, 2025
9 checks passed
@luanpotter luanpotter deleted the luan.custom-md branch March 12, 2025 13:38
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.

2 participants