search

mikepenz / multiplatform-markdown-renderer

Markdown renderer for Kotlin Multiplatform Projects (Android, iOS, Desktop), using Compose.
https://mikepenz.github.io/multiplatform-markdown-renderer/
Apache License 2.0
373 stars 25 forks source link

Android Compose library without Material #106

Closed yschimke closed 6 months ago

yschimke commented 8 months ago

About this issue

Markdown() composable has a strong dependency on material components like androidx.compose.material.MaterialTheme.

For Wear these should be avoided and instead use Wear Material versions.

See https://developer.android.com/training/wearables/compose#different

Is it feasible to pull these Color and Typography factories that default to mobile material to a separate dependency?

Details

0.11 Wear (Android)

Checklist

yschimke commented 8 months ago

n.b, FAQ is missing? https://github.com/mikepenz/multiplatform-markdown-renderer/blob/develop/FAQ.md

mikepenz commented 8 months ago

Good day @yschimke

Thank you very much for the report. The MaterialTheme is already only used in the default factories, allowing you to replace all of them with alternative variants:

If you adjust all of these, wouldn't R8 be capable of removing all references for Material classes?

yschimke commented 8 months ago

Yep, I'm not blocked. I would feel safer not relying on R8 to drop dependencies. Feel free to close if you feel strongly about it.

yschimke commented 8 months ago

And nice library.

mikepenz commented 8 months ago

@yschimke mostly curious on it. haven't shaped a decision on it yet. Do you have an example of a library which includes compose ui and splits to a wear module and other modules due to material theme?

yschimke commented 8 months ago

The whole of Wear Compose is like that :)

It has a Wear Material Text instead of Material Text. Wear MaterialTheme instead of MaterialTheme.

But honestly, I think it's about supporting more than just Material. There are other possible design systems, and I'd personally prefer to be able to import multiplatform-markdown-renderer without any explicit material assumptions. And a markdown-material for mobile, and markdown-wear for Wear.

mikepenz commented 7 months ago

Moving the MaterialTheme was done here now: https://github.com/mikepenz/multiplatform-markdown-renderer/pull/107/commits/16015cf2c5ba3c746d68d5d8ac45967d3aff7430

But how would you advice on going with Text or Surface in general. I have extracted those also by copying over some parts of Text and Surface from Material. However that doesn't seem very maintainable to do that in general? 🤔

https://github.com/mikepenz/multiplatform-markdown-renderer/pull/107

yschimke commented 7 months ago

I sort of got the impression you are making most of the decisions upfront, so using BasicText directly might be the right thing.

Wear Material actually doesn't have Surface at all. So maybe uplifting those to the m2 or m3 impls might be the solution?

Thanks for attempting this, I hope overall it's a clean up and more correct as people switch between m2, m3, wear.

mikepenz commented 7 months ago

Thank you for the additional input @yschimke

I would highly appreciate if you could review the PR https://github.com/mikepenz/multiplatform-markdown-renderer/pull/107

Decided to have wrappers (basically similar to what you mentioned on uplifting the m2/m3 impls) based on the normal surface/text components: https://github.com/mikepenz/multiplatform-markdown-renderer/pull/107/files#diff-8142270f21ac4367be27d783daec3cdff3fafe3e4e498bf9e046932a901b1dab - allowing the full removal of the Material dependency.

mikepenz commented 7 months ago

For everyone looking for this. There is a first pre-release available with the individual m2 and m3 artifact.

v0.13.0-a01 - https://github.com/mikepenz/multiplatform-markdown-renderer/tree/v0.13.0-a01