asjqkkkk / markdown_widget

📖Rendering markdown by flutter!Welcome for pr and issue.
MIT License
321 stars 93 forks source link

Chor: make ListNode extendable #133

Closed Maksimka101 closed 1 year ago

Maksimka101 commented 1 year ago

Hello,

Currently, inheriting from ListNode is not possible due to its private field _index, which the build method relies on.

Why do I want to extend ListNode? The motivation behind this is that it utilizes the Row with Expanded widgets, causing it to occupy all available horizontal space. I aim to rectify this behavior with the following code:

Row(
  mainAxisSize: MainAxisSize.min,
  crossAxisAlignment: CrossAxisAlignment.start,
  children: [
    SizedBox(
      width: space,
      child: marker,
    ),
    Flexible(
      child: Text.rich(
        TextSpan(
          children: [
            if (children.isNotEmpty) children.first.build(),
            for (final child in children.skip(1)) ...[
              // Introducing a new line before the next list item.
              // Otherwise, it might be rendered on the same line, disrupting the layout.
              if (child is UlOrOLNode) const TextSpan(text: '\n'),
              child.build(),
            ],
          ],
        ),
      ),
    ),
  ],
)

This code addresses the issue as follows:

By the way, should I create a pull request with the above fix? Or perhaps there's an alternative solution? What are your thoughts? 🙂

asjqkkkk commented 1 year ago

Hi @Maksimka101 , thanks for your pull request, here are my thoughts.

  1. If you only need to read the _index, there is no need to set it as public. You can just add a get method, which can avoid other people's wrong modifications.
  2. If you only use Flexible, it will get problems with error rendering. Therefore, if you want to modify it to Flexible, please also add the solution for \n text span.
Maksimka101 commented 1 year ago

Hey @asjqkkkk, I've made index variable private and introduced a getter

asjqkkkk commented 1 year ago

Hi @Maksimka101 , currently, the part about index seems to be fine but it's necessary to do some work with Flexible, otherwise, problems like the following will appear

image

So I think you need to include your solution in the pull request as well. then don't forget to replace Text.rich with ProxyRichText, and I may unify the configuration of Text.rich through ProxyRichText in the future.

Maksimka101 commented 1 year ago

You are right. Firstly, I only wanted to make the ListNode expandable and changed Expanded to Flexible by mistake. But, if you don't mind, I'll add the second fix

image