diff --git a/CHANGELOG.md b/CHANGELOG.md index 405beaaf..9b280a34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +# 4.2.2 +* Fixed `AsyncDrawable` display when it has placeholder with empty bounds ([#189]) +* Fixed `syntax-highlight` where code input is empty string ([#192]) +* Add `appendFactory`/`prependFactory` in `MarkwonSpansFactory.Builder` for more explicit `SpanFactory` ordering ([#193]) + +[#189]: https://github.com/noties/Markwon/issues/189 +[#192]: https://github.com/noties/Markwon/issues/192 +[#193]: https://github.com/noties/Markwon/issues/193 + # 4.2.1 * Fix SpannableBuilder `subSequence` method * Introduce Nougat check in `BulletListItemSpan` to position bullet (for bullets to be diff --git a/docs/docs/v4/core/spans-factory.md b/docs/docs/v4/core/spans-factory.md index 9567a788..e0b11aa1 100644 --- a/docs/docs/v4/core/spans-factory.md +++ b/docs/docs/v4/core/spans-factory.md @@ -62,7 +62,12 @@ builder.setFactory(Link.class, new SpanFactory() { --- -Since you can _add_ multiple `SpanFactory` for a single node: +:::warning +Deprecated in . Use `appendFactory` or `prependFactory` for +more explicit factories ordering. `addFactories` behaves like new `prependFactory` method. +::: + +Since you can _add_ multiple `SpanFactory` for a single node: ```java final Markwon markwon = Markwon.builder(context) @@ -81,6 +86,22 @@ final Markwon markwon = Markwon.builder(context) .build(); ``` +## appendFactory/prependFactory + +* use `appendFactory` if you wish to add a factory **after** original (can be used to post-process original factory) +* use `prependFactory` if you wish to add a factory **before** original (original factory will be applied after this one) + +```java +@Override +public void configureSpansFactory(@NonNull MarkwonSpansFactory.Builder builder) { + // `RemoveUnderlineSpan` will be added AFTER original, thus it will remove underline applied by original + builder.appendFactory(Link.class, (configuration, props) -> new RemoveUnderlineSpan()); + + // will be added BEFORE origin (origin can override this) + builder.prependFactory(Link.class, (configuration, props) -> new AbsoluteSizeSpan(48, true)); +} +``` + --- If you wish to inspect existing factory you can use: diff --git a/gradle.properties b/gradle.properties index b0b8871f..5b0fd422 100644 --- a/gradle.properties +++ b/gradle.properties @@ -8,7 +8,7 @@ android.enableJetifier=true android.enableBuildCache=true android.buildCacheDir=build/pre-dex-cache -VERSION_NAME=4.2.1 +VERSION_NAME=4.2.2 GROUP=io.noties.markwon POM_DESCRIPTION=Markwon markdown for Android diff --git a/markwon-core/src/main/java/io/noties/markwon/MarkwonSpansFactory.java b/markwon-core/src/main/java/io/noties/markwon/MarkwonSpansFactory.java index c4153075..43d2b1ec 100644 --- a/markwon-core/src/main/java/io/noties/markwon/MarkwonSpansFactory.java +++ b/markwon-core/src/main/java/io/noties/markwon/MarkwonSpansFactory.java @@ -39,10 +39,34 @@ interface Builder { * {@link SpanFactory} with the specified one. * * @since 3.0.1 + * @deprecated 4.2.2 consider using {@link #appendFactory(Class, SpanFactory)} or + * {@link #prependFactory(Class, SpanFactory)} methods for more explicit factory ordering. + * `addFactory` behaved like {@link #prependFactory(Class, SpanFactory)}, so + * this method call can be replaced with it */ @NonNull + @Deprecated Builder addFactory(@NonNull Class node, @NonNull SpanFactory factory); + /** + * Append a factory to existing one (or make the first one for specified node). Specified factory + * will be called after original (if present) factory. Can be used to + * change behavior or original span factory. + * + * @since 4.2.2 + */ + @NonNull + Builder appendFactory(@NonNull Class node, @NonNull SpanFactory factory); + + /** + * Prepend a factory to existing one (or make the first one for specified node). Specified factory + * will be called before original (if present) factory. + * + * @since 4.2.2 + */ + @NonNull + Builder prependFactory(@NonNull Class node, @NonNull SpanFactory factory); + /** * Can be useful when enhancing an already defined SpanFactory with another one. */ diff --git a/markwon-core/src/main/java/io/noties/markwon/MarkwonSpansFactoryImpl.java b/markwon-core/src/main/java/io/noties/markwon/MarkwonSpansFactoryImpl.java index ab2c1f84..fd7a99d5 100644 --- a/markwon-core/src/main/java/io/noties/markwon/MarkwonSpansFactoryImpl.java +++ b/markwon-core/src/main/java/io/noties/markwon/MarkwonSpansFactoryImpl.java @@ -56,7 +56,32 @@ public Builder setFactory(@NonNull Class node, @Nullable Spa @NonNull @Override + @Deprecated public Builder addFactory(@NonNull Class node, @NonNull SpanFactory factory) { + return prependFactory(node, factory); + } + + @NonNull + @Override + public Builder appendFactory(@NonNull Class node, @NonNull SpanFactory factory) { + final SpanFactory existing = factories.get(node); + if (existing == null) { + factories.put(node, factory); + } else { + if (existing instanceof CompositeSpanFactory) { + ((CompositeSpanFactory) existing).factories.add(0, factory); + } else { + final CompositeSpanFactory compositeSpanFactory = + new CompositeSpanFactory(factory, existing); + factories.put(node, compositeSpanFactory); + } + } + return this; + } + + @NonNull + @Override + public Builder prependFactory(@NonNull Class node, @NonNull SpanFactory factory) { // if there is no factory registered for this node -> just add it final SpanFactory existing = factories.get(node); if (existing == null) { diff --git a/markwon-core/src/main/java/io/noties/markwon/image/AsyncDrawable.java b/markwon-core/src/main/java/io/noties/markwon/image/AsyncDrawable.java index ce6f92d5..ad247238 100644 --- a/markwon-core/src/main/java/io/noties/markwon/image/AsyncDrawable.java +++ b/markwon-core/src/main/java/io/noties/markwon/image/AsyncDrawable.java @@ -164,20 +164,44 @@ public void setCallback2(@Nullable Callback cb) { @SuppressWarnings("WeakerAccess") protected void setPlaceholderResult(@NonNull Drawable placeholder) { // okay, if placeholder has bounds -> use it, otherwise use original imageSize + // it's important to NOT pass to imageSizeResolver when placeholder has bounds + // this is done, so actual result and placeholder can have _different_ + // bounds. Assume image is loaded with HTML and has ImageSize width=100%, + // so, even if placeholder has exact bounds, it will still be scaled up. + + // this condition should not be true for placeholder (at least for now) + // (right now this method is always called from constructor) + if (result != null) { + // but it is, unregister current result + result.setCallback(null); + } final Rect rect = placeholder.getBounds(); if (rect.isEmpty()) { - // if bounds are empty -> just use placeholder as a regular result - DrawableUtils.applyIntrinsicBounds(placeholder); + // check for intrinsic bounds + final Rect intrinsic = DrawableUtils.intrinsicBounds(placeholder); + if (intrinsic.isEmpty()) { + // @since 4.2.2 + // if intrinsic bounds are empty, use _any_ non-empty bounds, + // they must be non-empty so when result is obtained - proper invalidation will occur + // (0, 0, 1, 0) is still considered empty + placeholder.setBounds(0, 0, 1, 1); + } else { + // use them + placeholder.setBounds(intrinsic); + } + + // it is very important (if we have a placeholder) to set own bounds to it (and they must not be empty + // otherwise result won't be rendered) + // @since 4.2.2 + setBounds(placeholder.getBounds()); setResult(placeholder); + } else { - // this condition should not be true for placeholder (at least for now) - if (result != null) { - // but it is, unregister current result - result.setCallback(null); - } + // this method is not the same as above, as we do not want to trigger image-size-resolver + // in case when placeholder has exact bounds // placeholder has bounds specified -> use them until we have real result this.result = placeholder; diff --git a/markwon-core/src/test/java/io/noties/markwon/MarkwonSpansFactoryImplTest.java b/markwon-core/src/test/java/io/noties/markwon/MarkwonSpansFactoryImplTest.java index ad43e6e6..998ec152 100644 --- a/markwon-core/src/test/java/io/noties/markwon/MarkwonSpansFactoryImplTest.java +++ b/markwon-core/src/test/java/io/noties/markwon/MarkwonSpansFactoryImplTest.java @@ -113,6 +113,7 @@ public void composite_span_factory() { } @Test + @Deprecated public void builder_add_factory() { // here is what we should validate: // * if we call addFactory and there is none already -> supplied factory @@ -146,4 +147,74 @@ public void builder_add_factory() { assertEquals(compositeSpanFactory, builder.getFactory(node)); assertEquals(Arrays.asList(first, second, third), compositeSpanFactory.factories); } + + @Test + public void builder_prepend_factory() { + // here is what we should validate: + // * if we call prependFactory and there is none already -> supplied factory + // * if there is + // * * if not composite -> make composite + // * * if composite -> add to it + + final MarkwonSpansFactoryImpl.BuilderImpl builder = new MarkwonSpansFactoryImpl.BuilderImpl(); + + final SpanFactory first = mock(SpanFactory.class); + final SpanFactory second = mock(SpanFactory.class); + final SpanFactory third = mock(SpanFactory.class); + + final Class node = Node.class; + + // assert none yet + assertNull(builder.getFactory(node)); + + // add first, none yet -> it should be added without modifications + builder.prependFactory(node, first); + assertEquals(first, builder.getFactory(node)); + + // add second -> composite factory will be created + builder.prependFactory(node, second); + final MarkwonSpansFactoryImpl.CompositeSpanFactory compositeSpanFactory = + (MarkwonSpansFactoryImpl.CompositeSpanFactory) builder.getFactory(node); + assertNotNull(compositeSpanFactory); + assertEquals(Arrays.asList(first, second), compositeSpanFactory.factories); + + builder.prependFactory(node, third); + assertEquals(compositeSpanFactory, builder.getFactory(node)); + assertEquals(Arrays.asList(first, second, third), compositeSpanFactory.factories); + } + + @Test + public void builder_append_factory() { + // here is what we should validate: + // * if we call appendFactory and there is none already -> supplied factory + // * if there is + // * * if not composite -> make composite + // * * if composite -> add to it + + final MarkwonSpansFactoryImpl.BuilderImpl builder = new MarkwonSpansFactoryImpl.BuilderImpl(); + + final SpanFactory first = mock(SpanFactory.class); + final SpanFactory second = mock(SpanFactory.class); + final SpanFactory third = mock(SpanFactory.class); + + final Class node = Node.class; + + // assert none yet + assertNull(builder.getFactory(node)); + + // add first, none yet -> it should be added without modifications + builder.appendFactory(node, first); + assertEquals(first, builder.getFactory(node)); + + // add second -> composite factory will be created + builder.appendFactory(node, second); + final MarkwonSpansFactoryImpl.CompositeSpanFactory compositeSpanFactory = + (MarkwonSpansFactoryImpl.CompositeSpanFactory) builder.getFactory(node); + assertNotNull(compositeSpanFactory); + assertEquals(Arrays.asList(second, first), compositeSpanFactory.factories); + + builder.appendFactory(node, third); + assertEquals(compositeSpanFactory, builder.getFactory(node)); + assertEquals(Arrays.asList(third, second, first), compositeSpanFactory.factories); + } } \ No newline at end of file diff --git a/markwon-core/src/test/java/io/noties/markwon/core/CorePluginTest.java b/markwon-core/src/test/java/io/noties/markwon/core/CorePluginTest.java index 4137b83e..9cb95028 100644 --- a/markwon-core/src/test/java/io/noties/markwon/core/CorePluginTest.java +++ b/markwon-core/src/test/java/io/noties/markwon/core/CorePluginTest.java @@ -166,6 +166,18 @@ public MarkwonSpansFactory.Builder addFactory(@NonNull Class throw new RuntimeException(); } + @NonNull + @Override + public MarkwonSpansFactory.Builder appendFactory(@NonNull Class node, @NonNull SpanFactory factory) { + throw new RuntimeException(); + } + + @NonNull + @Override + public MarkwonSpansFactory.Builder prependFactory(@NonNull Class node, @NonNull SpanFactory factory) { + throw new RuntimeException(); + } + @Nullable @Override public SpanFactory getFactory(@NonNull Class node) { diff --git a/markwon-core/src/test/java/io/noties/markwon/image/AsyncDrawableTest.java b/markwon-core/src/test/java/io/noties/markwon/image/AsyncDrawableTest.java index 809cfd5f..d0a8f5c6 100644 --- a/markwon-core/src/test/java/io/noties/markwon/image/AsyncDrawableTest.java +++ b/markwon-core/src/test/java/io/noties/markwon/image/AsyncDrawableTest.java @@ -4,6 +4,7 @@ import android.graphics.ColorFilter; import android.graphics.Rect; import android.graphics.drawable.Drawable; + import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -18,7 +19,12 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @RunWith(RobolectricTestRunner.class) @Config(manifest = Config.NONE) @@ -78,6 +84,123 @@ public void previous_result_detached() { assertNotNull(result2.getCallback()); } + @Test + public void placeholder_no_bounds_no_intrinsic_bounds() { + // when there is a placeholder and its + // * bounds are empty + // * intrinsic bounds are empty + // AsyncDrawable.this must have any non-empty bounds (otherwise result won't be rendered, + // due to missing invalidation call) + + final Drawable placeholder = new AbstractDrawable() { + @Override + public int getIntrinsicWidth() { + return 0; + } + + @Override + public int getIntrinsicHeight() { + return 0; + } + }; + + assertTrue(placeholder.getBounds().isEmpty()); + + final AsyncDrawableLoader loader = mock(AsyncDrawableLoader.class); + when(loader.placeholder(any(AsyncDrawable.class))).thenReturn(placeholder); + + final AsyncDrawable drawable = new AsyncDrawable( + "", + loader, + mock(ImageSizeResolver.class), + null + ); + + final Rect bounds = drawable.getBounds(); + assertFalse(bounds.toShortString(), bounds.isEmpty()); + assertEquals(bounds.toShortString(), bounds, placeholder.getBounds()); + } + + @Test + public void placeholder_no_bounds_has_intrinsic() { + // placeholder has no bounds, but instead has intrinsic bounds + + final Drawable placeholder = new AbstractDrawable() { + @Override + public int getIntrinsicWidth() { + return 42; + } + + @Override + public int getIntrinsicHeight() { + return 24; + } + }; + + assertTrue(placeholder.getBounds().isEmpty()); + + final AsyncDrawableLoader loader = mock(AsyncDrawableLoader.class); + when(loader.placeholder(any(AsyncDrawable.class))).thenReturn(placeholder); + + final AsyncDrawable drawable = new AsyncDrawable( + "", + loader, + mock(ImageSizeResolver.class), + null + ); + + final Rect bounds = drawable.getBounds(); + assertFalse(bounds.isEmpty()); + assertEquals(0, bounds.left); + assertEquals(42, bounds.right); + assertEquals(0, bounds.top); + assertEquals(24, bounds.bottom); + + assertEquals(bounds, placeholder.getBounds()); + } + + @Test + public void placeholder_has_bounds() { + + final Rect rect = new Rect(0, 0, 12, 99); + final Drawable placeholder = mock(Drawable.class); + when(placeholder.getBounds()).thenReturn(rect); + + assertFalse(rect.isEmpty()); + + final AsyncDrawableLoader loader = mock(AsyncDrawableLoader.class); + when(loader.placeholder(any(AsyncDrawable.class))).thenReturn(placeholder); + + final AsyncDrawable drawable = new AsyncDrawable( + "", + loader, + mock(ImageSizeResolver.class), + null + ); + + final Rect bounds = drawable.getBounds(); + assertEquals(rect, bounds); + + verify(placeholder, times(1)).getBounds(); + verify(placeholder, never()).getIntrinsicWidth(); + verify(placeholder, never()).getIntrinsicHeight(); + verify(placeholder, never()).setBounds(any(Rect.class)); + } + + @Test + public void no_placeholder_empty_bounds() { + // when AsyncDrawable has no placeholder, then its bounds must be empty at the start + + final AsyncDrawable drawable = new AsyncDrawable( + "", + mock(AsyncDrawableLoader.class), + mock(ImageSizeResolver.class), + null + ); + + assertTrue(drawable.getBounds().isEmpty()); + } + private static class AbstractDrawable extends Drawable { @Override diff --git a/markwon-syntax-highlight/src/main/java/io/noties/markwon/syntax/Prism4jSyntaxHighlight.java b/markwon-syntax-highlight/src/main/java/io/noties/markwon/syntax/Prism4jSyntaxHighlight.java index cc78c401..922d2c56 100644 --- a/markwon-syntax-highlight/src/main/java/io/noties/markwon/syntax/Prism4jSyntaxHighlight.java +++ b/markwon-syntax-highlight/src/main/java/io/noties/markwon/syntax/Prism4jSyntaxHighlight.java @@ -41,6 +41,13 @@ protected Prism4jSyntaxHighlight( @NonNull @Override public CharSequence highlight(@Nullable String info, @NonNull String code) { + + // @since 4.2.2 + // although not null, but still is empty + if (code.isEmpty()) { + return code; + } + // if info is null, do not highlight -> LICENCE footer very commonly wrapped inside code // block without syntax name specified (so, do not highlight) return info == null diff --git a/sample/src/main/java/io/noties/markwon/sample/recycler/RecyclerActivity.java b/sample/src/main/java/io/noties/markwon/sample/recycler/RecyclerActivity.java index 815e5b03..84034ce9 100644 --- a/sample/src/main/java/io/noties/markwon/sample/recycler/RecyclerActivity.java +++ b/sample/src/main/java/io/noties/markwon/sample/recycler/RecyclerActivity.java @@ -2,9 +2,13 @@ import android.app.Activity; import android.content.Context; +import android.graphics.drawable.ColorDrawable; import android.net.Uri; import android.os.Bundle; +import android.text.TextPaint; import android.text.TextUtils; +import android.text.style.CharacterStyle; +import android.text.style.UpdateAppearance; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -13,6 +17,7 @@ import org.commonmark.ext.gfm.tables.TableBlock; import org.commonmark.node.FencedCodeBlock; +import org.commonmark.node.Link; import java.io.BufferedReader; import java.io.IOException; @@ -24,10 +29,14 @@ import io.noties.markwon.AbstractMarkwonPlugin; import io.noties.markwon.Markwon; import io.noties.markwon.MarkwonConfiguration; +import io.noties.markwon.MarkwonSpansFactory; import io.noties.markwon.MarkwonVisitor; import io.noties.markwon.core.CorePlugin; import io.noties.markwon.html.HtmlPlugin; -import io.noties.markwon.image.picasso.PicassoImagesPlugin; +import io.noties.markwon.image.ImagesPlugin; +import io.noties.markwon.image.file.FileSchemeHandler; +import io.noties.markwon.image.network.OkHttpNetworkSchemeHandler; +import io.noties.markwon.image.svg.SvgMediaDecoder; import io.noties.markwon.recycler.MarkwonAdapter; import io.noties.markwon.recycler.SimpleEntry; import io.noties.markwon.recycler.table.TableEntry; @@ -74,13 +83,14 @@ public void onCreate(@Nullable Bundle savedInstanceState) { private static Markwon markwon(@NonNull Context context) { return Markwon.builder(context) .usePlugin(CorePlugin.create()) -// .usePlugin(ImagesPlugin.create(plugin -> { -// plugin -// .addSchemeHandler(FileSchemeHandler.createWithAssets(context)) -// .addSchemeHandler(OkHttpNetworkSchemeHandler.create()) -// .addMediaDecoder(SvgMediaDecoder.create()); -// })) - .usePlugin(PicassoImagesPlugin.create(context)) + .usePlugin(ImagesPlugin.create(plugin -> { + plugin + .addSchemeHandler(FileSchemeHandler.createWithAssets(context)) + .addSchemeHandler(OkHttpNetworkSchemeHandler.create()) + .addMediaDecoder(SvgMediaDecoder.create()) + .placeholderProvider(drawable -> new ColorDrawable(0xFFff0000)); + })) +// .usePlugin(PicassoImagesPlugin.create(context)) // .usePlugin(GlideImagesPlugin.create(context)) // .usePlugin(CoilImagesPlugin.create(context)) // important to use TableEntryPlugin instead of TablePlugin @@ -106,10 +116,24 @@ public void configureVisitor(@NonNull MarkwonVisitor.Builder builder) { visitor.builder().append(code); }); } + + @Override + public void configureSpansFactory(@NonNull MarkwonSpansFactory.Builder builder) { + // `RemoveUnderlineSpan` will be added AFTER original, thus it will remove underline applied by original + builder.appendFactory(Link.class, (configuration, props) -> new RemoveUnderlineSpan()); + } }) .build(); } + private static class RemoveUnderlineSpan extends CharacterStyle implements UpdateAppearance { + + @Override + public void updateDrawState(TextPaint tp) { + tp.setUnderlineText(false); + } + } + @NonNull private static String loadReadMe(@NonNull Context context) { InputStream stream = null;