noties / Markwon

Android markdown library (no WebView)
https://noties.io/Markwon/
Apache License 2.0
2.77k stars 313 forks source link

[Question] Does Markwon have support for loose lists? #143

Closed mileskrell closed 5 years ago

mileskrell commented 5 years ago

I'd like to increase the spacing between my list items. See screenshot here.

Loose lists would work, but putting two line feeds between the items doesn't seem to have any effect in Markwon.

(Alternatively, if there isn't support for both tight and loose lists, does Markwon provide any configuration option for styling lists so I can change the spacing between the items?)

noties commented 5 years ago

Hello @mileskrell !

Loose lists would've worked if there was a special span registered for Paragraph nodes (markwon doesn't register one). The thing is, this issue was already discussed, take a look at #85 and then #116 . In short, obtain LastLineSpacingSpan and use it via SpanFactory:

final Markwon markwon = Markwon.builder(context)
        .usePlugin(new AbstractMarkwonPlugin() {
            @Override
            public void configureSpansFactory(@NonNull MarkwonSpansFactory.Builder builder) {
                // `addFactory` available from `3.0.1`
                builder.addFactory(ListItem.class, new SpanFactory() {
                    @Override
                    public Object getSpans(
                            @NonNull MarkwonConfiguration configuration,
                            @NonNull RenderProps props) {
                        return new LastLineSpacingSpan(128);
                    }
                });
            }
        })
        .build();

I'm going to add LastLineSpacingSpan in the next 4.0.0 version (as it seems pretty useful and can be used for Paragraph also)

mileskrell commented 5 years ago

Thanks for the fast response!

This is a lot better. The problem I'm seeing now is that since this extra space is added after every list item, it results in too much space being added at the end of nested list items.

Compare the following list:

with the same list displayed through Markwon.

There's too much space between "potato" and "minerals", because space is added after both "vegetables" and "potato".

I'm also noticing that there's not enough space between "vegetables" and "squash". If I compare this with the same list in Markwon without spacing customizations, it appears that the space between them isn't changed.


I think that the root of the problem is that the extra space after "vegetables" is added after its children. If the extra space were instead added before its children, I think that would solve both of these issues.

Do you know if there's any way of making that happen?

noties commented 5 years ago

Hello @mileskrell !

You are right, space is added after children (because we add space at the end of a span). As a workaround, we could add space before a span:

public class FirstLineSpacingSpan implements LineHeightSpan {

    private final int spacing;

    private int startAscent;
    private int startTop;

    public FirstLineSpacingSpan(@Px int spacing) {
        this.spacing = spacing;
    }

    @Override
    public void chooseHeight(CharSequence text, int start, int end, int spanstartv, int v, Paint.FontMetricsInt fm) {
        final int spanStart = ((Spanned) text).getSpanStart(this);
        if (start == spanStart) {

            // save these values, we will use them to restore fm state (if other lines are present)
            // if we do not, then all the subsequent lines will have space at the top
            this.startAscent = fm.ascent;
            this.startTop = fm.top;

            fm.ascent -= spacing;
            fm.top -= spacing;
        } else {
            // reset the values...
            fm.ascent = this.startAscent;
            fm.top = this.startTop;
        }
    }
}

If you would want to add space at the end of the whole list, you can still use LastLineSpacingSpan for BulletList/OrderedList nodes:

.usePlugin(new AbstractMarkwonPlugin() {
    @Override
    public void configureSpansFactory(@NonNull MarkwonSpansFactory.Builder builder) {
        builder
                .addFactory(ListItem.class, new SpanFactory() {
                    @Override
                    public Object getSpans(@NonNull MarkwonConfiguration configuration, @NonNull RenderProps props) {
                        return new FirstLineSpacingSpan(128);
                    }
                })
                .addFactory(BulletList.class, new SpanFactory() {
                    @Override
                    public Object getSpans(@NonNull MarkwonConfiguration configuration, @NonNull RenderProps props) {
                        return new LastLineSpacingSpan(128);
                    }
                });
    }
})
mileskrell commented 5 years ago

Thanks again! It's getting really close to what I'm looking for!

After some experimenting, it seems that I get the best results by using just FirstLineSpacingSpan:

builder.addFactory(ListItem.class, new SpanFactory() {
                            @Override
                            public Object getSpans(@NonNull MarkwonConfiguration configuration, @NonNull RenderProps props) {
                                return new FirstLineSpacingSpan(context, 60);
                            }
                        });

I'm now testing with the following string:

first line of text

* animals
* vegetables
  * cabbage
  * potatoes
    * red
    * brown
* minerals

second line of text

* animals
* vegetables
  * cabbage
  * potatoes
    * red
    * brown
* minerals

third line of text

In Markwon normally

In Markwon using FirstLineSpacingSpan as above

Everything within the lists looks great! There's only one issue: because I'm adding extra space above every list item, there's too much space before the first top-level element of both lists.


I can achieve the desired behavior if I modify FirstLineSpacingSpan#chooseHeight like so:

@Override
    public void chooseHeight(CharSequence text, int start, int end, int spanstartv, int v, Paint.FontMetricsInt fm) {
        final int spanStart = ((Spanned) text).getSpanStart(this);
        if (start == spanStart) {
            this.startAscent = fm.ascent;
            this.startTop = fm.top;
            if (spanStart != 20 && spanStart != 97) { // <-------- This check added
                fm.ascent -= spacingPx;
                fm.top -= spacingPx;
            }
        } else {
            fm.ascent = this.startAscent;
            fm.top = this.startTop;
        }
    }

Result: it works!

But of course, those numbers are specific to this string.

Is there any general way to perform this check? That is, is there any way of checking if the span is being applied to the first top-level element of a list?

noties commented 5 years ago

Hello @mileskrell !

Well, here is an idea - check if there is/are FirstLineSpacingSpan immediately before. If there is - add spacing, if there are none - it's the first list-item and no spacing should be added:

public class FirstLineSpacingSpan implements LineHeightSpan {

    private final int spacing;

    private int startAscent;
    private int startTop;

    public FirstLineSpacingSpan(@Px int spacing) {
        this.spacing = spacing;
    }

    @Override
    public void chooseHeight(CharSequence text, int start, int end, int spanstartv, int v, Paint.FontMetricsInt fm) {
        final int spanStart = ((Spanned) text).getSpanStart(this);
        if (start == spanStart) {

            // save these values, we will use them to restore fm state (if other lines are present)
            // if we do not, then all the subsequent lines will have space at the top
            this.startAscent = fm.ascent;
            this.startTop = fm.top;

            // obtain previous spans (if none -> we are first, no need to add spacing)
            // `-2` because there is a new-line character that won't have list-item span
            final FirstLineSpacingSpan[] spans =
                    ((Spanned) text).getSpans(start - 2, start, FirstLineSpacingSpan.class);

            if (spans != null && spans.length > 0) {
                fm.ascent -= spacing;
                fm.top -= spacing;
            }

        } else {
            // reset the values...
            fm.ascent = this.startAscent;
            fm.top = this.startTop;
        }
    }
}
mileskrell commented 5 years ago

Looks great! Thanks for all your help, and for all your work on this library!

ivancarras commented 2 years ago

This is Okay but IMO is not a completed solution, the best way is check PARAGRAPH_IS_IN_TIGHT_LIST prop and add the LineSpacing in that case:


class CustomListItemSpanFactory(
  @Px private val numberEndMargin: Int,
  @Px private val lineSpacingSpan: Int,
  private val drawable: Drawable // TODO
) : SpanFactory {

  override fun getSpans(configuration: MarkwonConfiguration, props: RenderProps): Any {
    return mutableListOf<Any>().apply {
      if (CoreProps.ListItemType.BULLET == CoreProps.LIST_ITEM_TYPE.require(props)) {
        add(
          CustomBulletListItemSpan(
            configuration.theme(),
            drawable,
            CoreProps.BULLET_LIST_ITEM_LEVEL.require(props)
          )
        )
      } else {
        // todo| in order to provide real RTL experience there must be a way to provide this string
        val number = CoreProps.ORDERED_LIST_ITEM_NUMBER.require(props).toString().plus(".")
        add(
          CustomOrderedListItemSpan(
            numberEndMargin,
            number
          )
        )
      }
      if (CoreProps.PARAGRAPH_IS_IN_TIGHT_LIST.require(props).not()) {
        add(LineSpacingSpan(lineSpacingSpan))
      }
    }.toTypedArray()
  }
}